Skip to content

Conversation

stephenLYZ
Copy link
Member

SUMMARY

This problem occurs when the user creates a native filter with sorting option, because the select component defaults to using localeCompare to sort when no sortComparator is passed, which overrides the original order. Here we fix it.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

before

Screen.Recording.2022-01-10.at.3.01.56.PM.mov

after

2022-01-23.10.39.35.mov

TESTING INSTRUCTIONS

ADDITIONAL INFORMATION

@codecov
Copy link

codecov bot commented Jan 23, 2022

Codecov Report

Merging #18145 (7ff09f5) into master (0cec0c9) will decrease coverage by 0.02%.
The diff coverage is 60.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #18145      +/-   ##
==========================================
- Coverage   65.95%   65.92%   -0.03%     
==========================================
  Files        1584     1587       +3     
  Lines       62046    62107      +61     
  Branches     6273     6284      +11     
==========================================
+ Hits        40920    40945      +25     
- Misses      19505    19541      +36     
  Partials     1621     1621              
Flag Coverage Δ
javascript 50.86% <60.00%> (-0.02%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...c/filters/components/Select/SelectFilterPlugin.tsx 68.42% <60.00%> (-0.60%) ⬇️
...rc/explore/components/controls/TextAreaControl.jsx 80.00% <0.00%> (-4.22%) ⬇️
...nd/src/explore/components/ExploreActionButtons.tsx 59.45% <0.00%> (-3.40%) ⬇️
...ews/CRUD/annotationlayers/AnnotationLayerModal.tsx 58.97% <0.00%> (-1.56%) ⬇️
superset-frontend/src/preamble.ts 0.00% <0.00%> (ø)
superset-frontend/src/views/App.tsx 0.00% <0.00%> (ø)
superset-frontend/src/setup/setupClient.ts 0.00% <0.00%> (ø)
...ackages/superset-ui-core/src/utils/featureFlags.ts 100.00% <0.00%> (ø)
.../superset-ui-core/src/connection/SupersetClient.ts 100.00% <0.00%> (ø)
...rset-ui-core/src/connection/SupersetClientClass.ts 100.00% <0.00%> (ø)
... and 14 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0cec0c9...7ff09f5. Read the comment docs.

@geido
Copy link
Member

geido commented Jan 24, 2022

/testenv up

@github-actions
Copy link
Contributor

@geido Ephemeral environment spinning up at http://34.217.78.34:8080. Credentials are admin/admin. Please allow several minutes for bootstrapping and startup.

@michael-s-molina
Copy link
Member

The problem of using a sortComparator that does nothing is when we unselect a value. It will keep the previously selected value at the top.

Screen.Recording.2022-01-25.at.10.50.36.AM.mov

@stephenLYZ
Copy link
Member Author

The problem of using a sortComparator that does nothing is when we unselect a value. It will keep the previously selected value at the top.

Screen.Recording.2022-01-25.at.10.50.36.AM.mov

Good catch! Adjusted.

Copy link
Member

@michael-s-molina michael-s-molina left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@github-actions
Copy link
Contributor

Ephemeral environment shutdown and build artifacts deleted.

shcoderAlex pushed a commit to casual-precision/superset that referenced this pull request Feb 7, 2022
…pache#18145)

* fix(native-filters): values is not sorted when setting sort option

* fix: revert

* pass sortComparator
ofekisr pushed a commit to nielsen-oss/superset that referenced this pull request Feb 8, 2022
…pache#18145)

* fix(native-filters): values is not sorted when setting sort option

* fix: revert

* pass sortComparator
bwang221 pushed a commit to casual-precision/superset that referenced this pull request Feb 10, 2022
…pache#18145)

* fix(native-filters): values is not sorted when setting sort option

* fix: revert

* pass sortComparator
@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 1.5.0 First shipped in 1.5.0 labels Mar 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels size/S 🚢 1.5.0 First shipped in 1.5.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants