Simplify dialog navigation to fix back button #23220
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
MindFreeze
added
WTH
|
Still have some work to do on alerts. The back button should work on them as well I guess |
|
Alright, I think it's in a pretty good state now. Ideally the |
bramkragten
reviewed
Dec 10, 2024
bramkragten
reviewed
Dec 10, 2024
| if ("dialog" in ev.state) { | ||
| // coming to a dialog | ||
| // in practice the dialog stack is empty when navigating forward, so this is a no-op | ||
| showDialogFromHistory(ev.state.dialog); |
There was a problem hiding this comment.
It is not really a no-op, as it will trigger a history.back (causing the forward button to be enabled again) that will then trigger a closeLastDialog which will no-op
There was a problem hiding this comment.
Yeah, I meant to say nothing will happen from a user perspective but no-op was shorter :D
bramkragten
approved these changes
Dec 10, 2024
9 tasks
boern99
pushed a commit
to boern99/ha_frontend
that referenced
this pull request
Dec 11, 2024
* Simplify dialog navigation to fix back btn * add back comment * manage dialog stack in the manager instead of history * handle dialogs that refuse to close
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Proposed change
Removed
replaceDialogand all related logic as it is not used anywhere. @bramkragten correct me if this is wrong but I couldn't find anything using it.This allowed me to simplify the history management logic for dialogs and fix back button issues.
Multiple modals on top of each other work well now.
Esccloses all dialogs but this is managed by the dialogs themselves via theescapeKeyActionattribute. An example of this is a siren with advanced options in themore-infodialog.Browser history was too limiting to do what we wanted. Only access to current state, no callbacks in the state, etc. So the dialog states are now managed in a stack inside
make-dialog-manager.tsand only a single state is pushed to the browser history.Going forward in history will not open a dialog anymore as it made no sense in most cases.
Type of change
Example configuration
Additional information
Checklist
If user exposed functionality or configuration variables are added/changed: