-
Notifications
You must be signed in to change notification settings - Fork 14.1k
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
feat(explore): each control can define its own canDrop for dnd #16090
Conversation
Codecov Report
@@ Coverage Diff @@
## master #16090 +/- ##
==========================================
- Coverage 76.86% 76.83% -0.04%
==========================================
Files 995 995
Lines 52876 52922 +46
Branches 6720 6738 +18
==========================================
+ Hits 40645 40660 +15
- Misses 12005 12037 +32
+ Partials 226 225 -1
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
/testenv up FEATURE_ENABLE_DRAG_AND_DROP=true |
@kgabryje Ephemeral environment spinning up at http://35.167.114.77:8080. Credentials are |
@kgabryje I didn't understand why we need to create an additional function for this. |
@michael-s-molina We can only check control's own value in |
@villebro What do you think? I'm still not sure if we need this extra prop. |
I agree with Kamil's assessment and see this as improving decoupling. By adding this we can let both components decide about the things they have the best context on. Without it we face a situation where we would need to make control1 aware of logic that should be inside control2 and vice versa. |
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.
Thanks for the clarifications!
Ephemeral environment shutdown and build artifacts deleted. |
…e#16090) * feat(explore): each control can define its own canDrop for dnd * Make canDropValue optional * Add onDropValue
…e#16090) * feat(explore): each control can define its own canDrop for dnd * Make canDropValue optional * Add onDropValue
SUMMARY
Allows defining
canDropValue
function inmapStateToProps
of controls in control panel (in superset-ui).That will allow us not only to use generic
canDrop
functions for columns or adhoc metrics, but also more advanced ones - likecan drop column "gender" in "Series" control only if "Breakdown" control doesn't already have this column added
.It's a prerequisite for fixing issue #16071
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
No changes
TESTING INSTRUCTIONS
ADDITIONAL INFORMATION
CC: @junlincc