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

Replace redux-responsive with a custom simpler alternative #3878

Merged
merged 1 commit into from
Dec 8, 2017

Conversation

youknowriad
Copy link
Contributor

closes #3875

This PR removes the redux-responsive module which was not compatible IE11 and replace it with a simpler custom alternative.

Testing instructions

  • Leave the sidebar open
  • Resize the window to make it "mobile" like
  • The sidebar should close when you reach the mobile breakpoint

@aduth
Copy link
Member

aduth commented Dec 8, 2017

Is this something we could fix in redux-responsive? I like the approach they use which is based on event listeners on matchMedia, not polling.

https://developer.mozilla.org/en-US/docs/Web/API/MediaQueryList/addListener

@youknowriad
Copy link
Contributor Author

I think we'd want to avoid store enhancers regardless of this. we might want to mimic their event listeners ( see #3832 )

@youknowriad
Copy link
Contributor Author

@aduth Do you think we can get this in for the release (to avoid breakage) and improve the internals to use matchMedia as a follow-up?

@afercia
Copy link
Contributor

afercia commented Dec 8, 2017

Tested in IE11 no error 🙂 Breakpoint works.

@youknowriad youknowriad merged commit e4707b9 into master Dec 8, 2017
@youknowriad youknowriad deleted the update/mobile-state branch December 8, 2017 17:45
} );
}, 100 );

window.addEventListener( 'resize', updateSize );
Copy link
Member

@gziolo gziolo Dec 8, 2017

Choose a reason for hiding this comment

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

I’m wondering what will happen with those event listeners when Redux store gets recreated after an error that crashes editor. It would be great to double check they don’t get cloned every time it happens.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

IE!!: blanck page and JS error "Object.keys: argument is not an object"
4 participants