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
chore: Scrolls top when opening a select filter #15141
chore: Scrolls top when opening a select filter #15141
Conversation
Codecov Report
@@ Coverage Diff @@
## master #15141 +/- ##
=======================================
Coverage 77.26% 77.26%
=======================================
Files 969 969
Lines 49918 49924 +6
Branches 6393 6395 +2
=======================================
+ Hits 38567 38573 +6
Misses 11148 11148
Partials 203 203
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
@@ -275,14 +292,15 @@ export default function PluginFilterSelect(props: PluginFilterSelectProps) { | |||
onSearch={searchWrapper} | |||
onSelect={clearSuggestionSearch} | |||
onBlur={handleBlur} | |||
onDropdownVisibleChange={setIsDropdownVisible} | |||
onDropdownVisibleChange={onDropdownVisibleChange} |
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.
I think we can use a different (maybe a bit simpler?) approach here
onDropdownVisibleChange={onDropdownVisibleChange} | |
dropdownRender={originNode => { | |
if (isDropdownVisible && !wasDropdownVisible) { | |
originNode.ref?.current?.scrollTo(0); | |
} | |
return originNode; | |
}} |
and before that:
const wasDropdownVisible = usePrevious(isDropdownVisible);
What do you think?
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.
I think it's a really great solution. Much more elegant! I'll make the changes and test it. Thank you!
a665a1c
to
3d6c6d2
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
SUMMARY
Fixes #15029
@rusackas @junlincc @villebro
@geido If we add the
inverseSelection
prop to the new Select, then this logic will be needed.BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
121098597-10325080-c7ab-11eb-88fb-dca5d032959d_kUroVy7a.mov
screen-recording-2021-06-13-at-25900-pm_cGjyCPjl.mov
TESTING INSTRUCTIONS
See before/after videos for instructions.
ADDITIONAL INFORMATION