rusackas commented on code in PR #44858:
URL: https://github.com/apache/superset/pull/44858#discussion_r4177521095
##########
superset-frontend/src/components/Datasource/components/CollectionTable/index.tsx:
##########
@@ -127,15 +127,32 @@ export default function CRUDCollection({
: 10,
);
const [currentPage, setCurrentPage] = useState<number>(1);
+ // The row order to restore when a sort is cleared. Tracked from
+ // collectionArray itself (not propsCollection) so it reflects in-progress
+ // edits whose onChange round trip to the parent hasn't landed back in
+ // props yet.
+ const unsortedOrderRef = useRef<Array<string | number>>(
+ initialKeyed.current!.collectionArray.map(item => item.id),
+ );
// Sync with props.collection changes
useEffect(() => {
const { collection: newCollection, collectionArray: newCollectionArray } =
createKeyedCollection(propsCollection);
setCollection(newCollection);
setCollectionArray(newCollectionArray);
+ // Refresh the restore order too, so that clearing a sort after an
+ // external sync (e.g. a source-control-synced column set) reflects the
+ // synced order and row set instead of stale, pre-sync ids.
+ unsortedOrderRef.current = newCollectionArray.map(item => item.id);
Review Comment:
Good catch @aminghadersohi. The sync effect now skips the restore-order
refresh when the incoming row order matches what is already displayed while
sorted, so an echoing parent no longer overwrites it. Added a
round-tripping-parent test that fails without the change.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
