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: fix history browsing with nested Reveal and deep-linking #11093

Merged

Conversation

ncoden
Copy link
Contributor

@ncoden ncoden commented Mar 26, 2018

Changes:

  • update history when closing Reveal with replaceState
    When closing a Reveal with replaceState: true, push a new entry to reset the hash. So going back to the history reopen the modals opened before in reverse order.
  • do not update history when opening Reveal with its hash already set (like when going back in history)
  • add visual tests for nested Reveal with deep-linking

Closes: #8012
Visual tests are from: https://codepen.io/DanielRuf/pen/OvPpzb

…n#8012

When closing a Reveal with `replaceState: true`, push a new entry to reset the hash. So going back to the history reopen the modals opened before in reverse order.

Closes foundation#8012
Copy link
Contributor

@DanielRuf DanielRuf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Found no issues 👍

@ncoden
Copy link
Contributor Author

ncoden commented Mar 26, 2018

Thank you for the review

@ncoden ncoden merged commit 7ea9dde into foundation:develop Mar 27, 2018
@ncoden ncoden deleted the fix/nested-reveal-deep-link-history-8012 branch April 7, 2018 17:45
ncoden added a commit to ncoden/foundation-sites that referenced this pull request Jun 16, 2018
…link-history-8012 for v6.5.0

42d267a fix: update history when closing Reveal with `replaceState` foundation#8012
88c08bd fix: prevent Reveal opening with its hash already set to add history entry
d5fe114 tests: add visual tests for nested Reveal with deep-linking

Signed-off-by: Nicolas Coden <nicolas@ncoden.fr>
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.

None yet

2 participants