rusackas commented on code in PR #44858:
URL: https://github.com/apache/superset/pull/44858#discussion_r4178359812


##########
superset-frontend/src/components/Datasource/components/CollectionTable/index.tsx:
##########
@@ -341,24 +375,38 @@ export default function CRUDCollection({
             return mStr.localeCompare(nStr);
           };
 
+          // Sort the live, edited collection rather than propsCollection, so
+          // an edit that hasn't round-tripped back through onChange yet

Review Comment:
   Good catch, @sadpandajoe. Descending now uses a negated comparator instead 
of `reverse()`, and a pagination event that re-emits the current sorter no 
longer re-sorts. Added a 30-row, page-size-25 test with blank labels that 
checks all rows show up across both pages.



##########
superset-frontend/src/components/Datasource/components/CollectionTable/index.tsx:
##########
@@ -341,24 +375,38 @@ export default function CRUDCollection({
             return mStr.localeCompare(nStr);
           };
 
+          // Sort the live, edited collection rather than propsCollection, so
+          // an edit that hasn't round-tripped back through onChange yet
+          // isn't dropped when a sort is applied.
+          sortedArray = [...collectionArray];
           sortedArray.sort((a: CollectionItem, b: CollectionItem) =>
             compareSort(a[col] as Sort, b[col] as Sort),
           );
           if (newSortOrder === SortOrderEnum.Desc) {
             sortedArray.reverse();
           }
         } else {
-          const { collectionArray: resetArray } =
-            createKeyedCollection(propsCollection);
-          sortedArray = resetArray;
+          // Restore the pre-sort order, but take each row's current value
+          // from the live `collection` map (not propsCollection) so an edit
+          // made while sorted survives clearing the sort. Any row not part
+          // of the tracked order (e.g. added while sorted) is appended.
+          const trackedIds = new Set(unsortedOrderRef.current);
+          sortedArray = unsortedOrderRef.current
+            .filter(id => collection[id])
+            .map(id => collection[id]);
+          collectionArray.forEach(item => {
+            if (!trackedIds.has(item.id)) {

Review Comment:
   Thanks @aminghadersohi. Added a test that adds a row while sorted, clears 
the sort, and checks the row is still there.



-- 
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]

Reply via email to