-
Notifications
You must be signed in to change notification settings - Fork 14k
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(sqllab): Table options rendering regression #24142
fix(sqllab): Table options rendering regression #24142
Conversation
@@ -440,6 +441,11 @@ const Select = forwardRef( | |||
onChange?.(newValues, newOptions); | |||
}; | |||
|
|||
const shouldUseChildrenOptions = useMemo( | |||
() => !selectAllEnabled || hasCustomLabels(options), |
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.
() => !selectAllEnabled || hasCustomLabels(options), | |
() => selectAllEnabled || hasCustomLabels(options), |
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
Codecov Report
@@ Coverage Diff @@
## master #24142 +/- ##
==========================================
- Coverage 68.28% 68.26% -0.02%
==========================================
Files 1952 1952
Lines 75432 75353 -79
Branches 8191 8207 +16
==========================================
- Hits 51505 51437 -68
+ Misses 21823 21810 -13
- Partials 2104 2106 +2
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 3 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
(cherry picked from commit 6a44e0d)
SUMMARY
There's a rendering perf regression from #22084
When SelectAll feature introduced by #22084
, Select component became rendering options by jsx only. This makes the performance degraded for the large size options.
This commit recovers the way to use options prop as before (including SelectAll condition)
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
After: (~100ms)
Before: (>5000ms)
TESTING INSTRUCTIONS
select a large size table option schema
click the table dropdown and then measure the delay
ADDITIONAL INFORMATION