-
Notifications
You must be signed in to change notification settings - Fork 14.4k
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: set label on adhoc column should persist #26154
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #26154 +/- ##
==========================================
+ Coverage 69.13% 69.20% +0.07%
==========================================
Files 1941 1941
Lines 75893 75901 +8
Branches 8450 8453 +3
==========================================
+ Hits 52466 52531 +65
+ Misses 21252 21177 -75
- Partials 2175 2193 +18
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
/testenv up |
@eschutho Ephemeral environment spinning up at http://18.237.178.227:8080. Credentials are |
Ephemeral environment shutdown and build artifacts deleted. |
(cherry picked from commit b2ea97a)
(cherry picked from commit b2ea97a)
(cherry picked from commit b2ea97a)
SUMMARY
The component for defining columns has an unorthodox architecture:
When a user edits the label using the pencil in
<ColumnSelectPopoverTrigger />
, the<ColumnSelectPopover />
component is not aware of that change. If the users then clicks "SAVE" to use the adhoc column ("CUSTOM SQL") with the new label, the<ColumnSelectPopover />
won't do anything because it's not aware of any changes.This PR modifies
<ColumnSelectPopover />
so that if the user selects "CUSTOM SQL" and the label has been modified (via a new prophasCustomLabel
), then an adhoc column is defined in the component's state. This way, when "SAVE" is clicked, the label gets persisted correctly.BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
TESTING INSTRUCTIONS
ADDITIONAL INFORMATION