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

Hidden does not work after window.addEventListener('resize', this.resizeEvent). #12798

Closed
Taz742 opened this issue Sep 7, 2018 · 7 comments
Closed
Labels
bug 🐛 Something doesn't work good first issue Great for first contributions. Enable to learn the contribution process.

Comments

@Taz742
Copy link

Taz742 commented Sep 7, 2018

Hello, i want to use a Hidden property, but when I'm using window.addEventListener('resize', this.resizeEvent) inside my component Hidden does not work... Any idea how can I use both of them?

Tech Version
Material-UI ^1.5.1
React 16.4.2
Browser Google chrome
TypeScript
etc.
@oliviertassinari oliviertassinari added the status: waiting for author Issue with insufficient information label Sep 7, 2018
@oliviertassinari
Copy link
Member

@Taz742 Please provide a full reproduction test case. This would help a lot 👷 .
A live example would be perfect. This codesandbox.io template may be a good starting point. Thank you!

@Taz742
Copy link
Author

Taz742 commented Sep 7, 2018

@oliviertassinari https://codesandbox.io/s/yqnzq2jk31
Please comment
this.setState({
check: !mediaQuery.matches,
})
this 3 lines in resizeEvent function and try again to resize browser, now you can see problem.

@oliviertassinari oliviertassinari added bug 🐛 Something doesn't work good first issue Great for first contributions. Enable to learn the contribution process. and removed status: waiting for author Issue with insufficient information labels Sep 7, 2018
@oliviertassinari
Copy link
Member

oliviertassinari commented Sep 7, 2018

@Taz742 It's exactly the same issue than with #12288. The fix is the same in withWidth, I hope it can help with performance too:
https://github.com/mui-org/material-ui/blob/11c340c6ef8ca2a469dcaf626c4a0fc2d479d366/packages/material-ui/src/withWidth/withWidth.js#L127-L129

- <EventListener target="window" onResize={this.handleResize}> 
-   <Component {...more} /> 
- </EventListener> 
+      <React.Fragment>
+        <Component {...more} /> 
+        <EventListener target="window" onResize={this.handleResize} />
+      </React.Fragment>

@oliviertassinari
Copy link
Member

@Taz742 Do you want to work on it?

@Taz742
Copy link
Author

Taz742 commented Sep 8, 2018

@Taz742
Copy link
Author

Taz742 commented Sep 8, 2018

@oliviertassinari Yes sure, i want to work on it. Thanks!
What was the problem in my example?

@oliviertassinari
Copy link
Member

oliviertassinari commented Sep 8, 2018

@Taz742 The problem is that you are calling setState, it's rerendering the React tree, unsubscribing and resubscribing to the resize event. In the process, we miss the event. By pruning the rerendering of EventListener, we can respond to the event.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Something doesn't work good first issue Great for first contributions. Enable to learn the contribution process.
Projects
None yet
Development

No branches or pull requests

2 participants