-
Notifications
You must be signed in to change notification settings - Fork 8.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
[Embeddable Rebuild] [Controls] Apply selections on reset #189580
Comments
Pinging @elastic/kibana-presentation (Team:Presentation) |
One potential path forward Each control must expose a Then control group |
This wouldn't be such a problem if we could synchronously transform selections into filters. Because we can't, the above solution is basically introducing an asynchronous state transition which can get very complicated. For instance. Will the Dashboard's To be completely aligned here, would we need to make the Can we think of another way to do this? |
I explored @ThomThomson's suggestion of a So we're back to what you suggested @nreese - however, with a few small details:
|
I've created a draft PR against the unsaved changes PR here to show off what this could look like: nreese#41 (the types are still angry - would need to figure that out) |
Thanks for pathfinding a solution, this looks promising. Each control would need to What is the plan with this? Are we waiting until #189128 merges and then working on a solution? Or are we trying to incorporate a solution into #189128? |
@nreese Up to you! I'm okay either way - what would be easiest for you? :) |
I think after is preferred so we can focus on as small a problem is possible without all the noise of the entire unsaved implementation. I think there will be lots of iterating on the fix, so it would be best to do this in isolation. |
#189128 introduces a few changes in behaviour for the control group apply button compared to the version currently on
main
. To summarize these changes:apply
button enabled.Because of (1) specifically, this creates an awkward user experience where, on dashboard reset, the user must also click "apply" in order to see the true unsaved state of the dashboard:
Screen.Recording.2024-07-30.at.3.14.03.PM.mov
If I am resetting my dashboard, I am expecting it to reset all my state to the last saved state - but this breaks that pattern because I have to hit “reset” and “apply” to see my true last saved state. So, we need to fix this before the React control group is ready to replace the legacy control group.
The text was updated successfully, but these errors were encountered: