-
Notifications
You must be signed in to change notification settings - Fork 13.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: Order of Select items when unselecting #17169
fix: Order of Select items when unselecting #17169
Conversation
Codecov Report
@@ Coverage Diff @@
## master #17169 +/- ##
==========================================
+ Coverage 76.91% 76.94% +0.02%
==========================================
Files 1038 1039 +1
Lines 55557 55592 +35
Branches 7567 7584 +17
==========================================
+ Hits 42731 42774 +43
+ Misses 12576 12568 -8
Partials 250 250
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
As discussed, I think it would be great to have a central place where the sorting functions can be defined to avoid repetition in the code. |
That's a great idea! We can have functions that take into consideration a specific property, handle numeric types, etc. |
/testenv up |
@yousoph Ephemeral environment spinning up at http://54.189.67.76:8080. Credentials are |
Tested a bit on the ephemeral, looking good to me. Thanks @michael-s-molina ! |
dcfb468
to
bd3f4e9
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Thanks for the improvement!
Ephemeral environment shutdown and build artifacts deleted. |
SUMMARY
The objective of this PR is to fix an issue with the order of select options when unselecting. It also changes the default sorting algorithm of the
Select
component to alphabetical sorting while providing an alternative to the component's users to override this behavior. Alphabetical sorting is the most common sorting algorithm used by select components in Superset but we do have cases where the order is calculated by different criteria. Another objective of the solution is to enable server-side sorting.To fill these objectives, this PR introduced an optional property to the
Select
calledsortCompator
that accepts a comparison function and is set to alphabetical comparison by default. This function accepts any object that adheres to the select options interface which allows custom properties to be used when comparing the options. This can be used by server-side sorting, where the response can include anorder
field or similar to be used in a custom sort comparator.This comparator is also important to reorder the items after an unselect action because we don't know the original order of the elements, especially with server-side sorting.
This PR also:
@geido Can you include this information in the Select API docs?
@yousoph @rusackas @hughhhh @junlincc
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
Screen.Recording.2021-10-20.at.11.04.20.AM.mov
Screen.Recording.2021-10-20.at.11.06.25.AM.mov
TESTING INSTRUCTIONS
Check the videos for instructions
ADDITIONAL INFORMATION