1188sort #1374
1188sort #1374
Conversation
…to localeCompare() on some files (still needs testing
…ortBy() to localeCompare() on some files (still needs testing" This reverts commit 8d2c2d4.
…) to localeCompare(), tested by comparing the group and meter options when creating a new group
…o localeCompare()
… graphic units on create a group page (might need further testing)
…he generated csv files of graphs; removed commented out old code
…CompatibleUnits, input
…elector() called in top level component rather than separately in functions
…elector() called in top level component rather than separately in functions
… selectMeterGroupSelectData() and selectUnitSelectData() to include selectSelectedLanguage in appSelector
…erMenuOptionsForGroup(), updated useEffects for CreateGroupModalComponent() and EditGroupModalComponent() to include locale as arguments.
… updated selectPossibleGraphicUnits() in adminSelectors to include language as an argument, cleaned up TODO and typecasted locale into String for determinecompatibleunits for getMeterMenuOptionsForGroup
huss
left a comment
huss
left a comment
There was a problem hiding this comment.
Thanks to @gyordong for quickly and carefully addressing all the previous comments. All the changes look good and testing found no issues. I did carefully search for all uses of .sort and found a few that do not seem language aware but maybe should be. They are:
- src/client/app/components/groups/EditGroupModalComponent.tsx: has two .sort that are not locale aware
- src/client/app/components/conversion/ConversionsDetailComponent.tsx: has one .sort that are not locale aware
- src/client/app/components/admin/users/UsersDetailComponent.tsx: has one .sort that are not locale aware
- src/client/app/components/MultiCompareChartComponent.tsx: has a manual ordered sort of identifier that does not seem locale aware.
Do you think they should be? I'm hoping all but the last one are fairly straightforward if a change is needed. I know this potentially means more changes but it might get the entire code base in a great place. I appreciate your consideration and I'm happy to help as you wish.
|
I've implemented all of your requests except for MultiCompareChartComponent, as only Alphabetical uses a string comparison and the other two sortingorders, ascending and descending, which sorts it off of the change of the unit and are numerical. |
Yes, that was what I was thinking. Only the string one should be modified (as you did). |
3 participants
Description
My partner @aduques and I changed some functions using sortBy() to use localeCompare() instead. This allowed text with accents in them to be order properly (i.e, before changes it was caa, cna, cáa, cña, now it's properly ordered as caa, cáa, cna, cña)
Fixes #1188
Type of change
Checklist
Limitations
Some files, specifically uiSelectors, determineCompatibleUnits, and input, have undefined set as the locale argument for localeCompare(). The alphabetic ordering for accents worked for all languages with undefined set, but we can change it to use the selected language if deemed necessary. We left TODOs on these specific files for later developers to locate and change if it ever becomes an issue in the future.