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.

Sign up for GitHub

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

Jump to bottom

fix: use hashchange event instead of popstate for IE/Edge compatibility #11011

Merged
merged 1 commit into from
Mar 6, 2018
Merged

fix: use hashchange event instead of popstate for IE/Edge compatibility #11011

merged 1 commit into from
Mar 6, 2018

Conversation

ncoden
Copy link
Contributor

@ncoden ncoden commented Mar 4, 2018 •
edited
Loading

We do not even need to listen on all history change. We currently use popstate (checked for Accordion, Magellan, Reveal and Tabs) only for window.location.hash, and could rely on hashchange instead without any problems. No need for a polyfill.

See: #11006 (comment)

Events:

Updated components:

  • Accordion
  • Magellan
  • Reveal
  • Tabs

Previous PR: #11006 (@DanielRuf)
Closes #10771 (@hubert-ried)

@ncoden ncoden mentioned this pull request Mar 4, 2018
Closed
@ncoden ncoden requested a review from DanielRuf March 4, 2018 16:35
@ncoden ncoden added this to the 6.4.4 milestone Mar 4, 2018
DanielRuf
DanielRuf approved these changes Mar 4, 2018
View reviewed changes
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.

Should be sufficient as we do not have to listen for any history changes, just to hash changes.

@ncoden
Copy link
Contributor Author

ncoden commented Mar 4, 2018 •
edited
Loading

@DanielRuf Yes this is the reason of this PR. We currently only need to listen on hash changes. However, we would need a polyfill for popstate.

@ncoden ncoden merged commit cb3da32 into foundation:develop Mar 6, 2018
@ncoden ncoden deleted the fix/hashchange-event-compatibility-10771 branch April 7, 2018 17:44
ncoden added a commit to ncoden/foundation-sites that referenced this pull request Jun 16, 2018
@ncoden
Use pull request foundation#11011 from ncoden/fix/hashchange-event-co…
6a293df 
…mpatibility-10771 for v6.5.0

4fbcb91 fix: use `hashchange` event instead of `popstate` for IE/Edge compatibility

Signed-off-by: Nicolas Coden <[email protected]>
This was referenced Jun 16, 2018
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@DanielRuf DanielRuf DanielRuf approved these changes

Assignees
No one assigned
Labels
bug fix Tabs
Projects
None yet
Milestone
6.5.0
Development

Successfully merging this pull request may close these issues.
2 participants
@ncoden @DanielRuf