[GitHub] [superset] corbinrobb commented on a change in pull request #17638: fix(select): [WIP] select component sort functionality on certain options

[GitBox <gi...@apache.org>]

corbinrobb commented on a change in pull request #17638:
URL: https://github.com/apache/superset/pull/17638#discussion_r762135198



##########
File path: superset-frontend/src/components/Select/Select.tsx
##########
@@ -289,7 +303,9 @@ const Select = ({
   pageSize = DEFAULT_PAGE_SIZE,
   placeholder = t('Select ...'),
   showSearch = true,
-  sortComparator = defaultSortComparator,
+  sortByProperty = 'label',
+  sortComparator = propertyComparator(sortByProperty),
+  sortOptions = false,

Review comment:
       Yeah it doesn't feel right. I can change that to true, I only have it like this because in the majority (I only have found one that isn't) of the places I found this component being used it is being passed a list that is in order. And a lot of them don't have a value that can easily be sorted upon. If I have it set as true as the default then I will need to go and set every static number list being passed in to sort by its value instead because it has been sorting them by default and that is what was unsorting them. 
   
   I figured that since most were like that then the default should reflect that but I may very well not be seeing all of them. I also can't think of a way that we can sort them appropriately without have to go manually add the prop. Do you have any thoughts on that? Would you like me to change it to true and add the property to every configuration where it should be sorted?




-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org