Skip to content
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 navigation from stacked dialogs with the same name #23698

Merged
merged 3 commits into from
Jan 14, 2025
Merged

Conversation

MindFreeze
Copy link
Contributor

Proposed change

When 2 the same dialog appears twice in the stack, the first closeDialog call closes both, so the second one is missing from the stack when the loop gets to it. Not sure the closing behavior should be changed but for the linked bug, checking the state is enough.

Type of change

  • Dependency upgrade
  • Bugfix (non-breaking change which fixes an issue)
  • New feature (thank you!)
  • Breaking change (fix/feature causing existing functionality to break)
  • Code quality improvements to existing code or addition of tests

Example configuration

Additional information

Checklist

  • The code change is tested and works locally.
  • There is no commented out code in this PR.
  • Tests have been added to verify that the new code works.

If user exposed functionality or configuration variables are added/changed:

@MindFreeze MindFreeze changed the title Dialog fix Fix navigation from stacked dialogs with the same name Jan 12, 2025
@bramkragten
Copy link
Member

bramkragten commented Jan 13, 2025

Wouldn't it be better to make sure a dialog can never be in the open stack more than once? As it is technically not possible to have the same dialog open twice...

@MindFreeze
Copy link
Contributor Author

then you won't be able to navigate back to the previous more info dialog but this is not possible even now so I guess you're right

@MindFreeze
Copy link
Contributor Author

Added a check so a dialog can only be in the stack once. Still left the check in closeAll though because we aren't using forEach so the array item is not guaranteed to be there

@bramkragten
Copy link
Member

We should btw clean up this stuff:

diff --git a/src/dialogs/more-info/ha-more-info-dialog.ts b/src/dialogs/more-info/ha-more-info-dialog.ts
index c37e337aa..ffb46c998 100644
--- a/src/dialogs/more-info/ha-more-info-dialog.ts
+++ b/src/dialogs/more-info/ha-more-info-dialog.ts
@@ -180,16 +180,6 @@ export class MoreInfoDialog extends LitElement {
   }
 
   private _setView(view: View) {
-    history.replaceState(
-      {
-        ...history.state,
-        dialogParams: {
-          ...history.state?.dialogParams,
-          view,
-        },
-      },
-      ""
-    );
     this._currView = view;
   }
 

@bramkragten bramkragten added this to the 2025.1 milestone Jan 14, 2025
@bramkragten bramkragten merged commit 6f9a385 into dev Jan 14, 2025
15 checks passed
@bramkragten bramkragten deleted the dialog-fix branch January 14, 2025 08:25
@RMMTSLLP
Copy link

Hi Guys, The issue still appears to be still here after upgrading to 2025.1.3 was the fix implemented in this version?

@MindFreeze
Copy link
Contributor Author

@RMMTSLLP This fix hasn't been released yet

piitaya pushed a commit that referenced this pull request Jan 23, 2025
* Fix navigation from stacked dialogs

* lint fix

* Keep only 1 instance per dialog tag in the stack
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Link to edit automation from entity dropdown menu "Related" no longer working
4 participants