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

Add matchMedia listener based alternative to Hide component #396

Open
jxnblk opened this issue Oct 19, 2018 · 6 comments
Open

Add matchMedia listener based alternative to Hide component #396

jxnblk opened this issue Oct 19, 2018 · 6 comments

Comments

@jxnblk
Copy link
Contributor

jxnblk commented Oct 19, 2018

Based on some discussions with @sdalonzo, it might make sense to include an alternative to the Hide component that would allow for conditional rendering based on media query, rather than relying on CSS display: none. This might make use of the context API to use a global listener and context consumers to control rendering.

Other libraries for reference:

@damassi
Copy link

damassi commented Oct 20, 2018

@jxnblk and @sdalonzo - We've been having a similar discussion at Artsy about approaches to this, as we currently use conditional rendering but have been debating going the other direction towards display: none. Wondering if y'all ran into any issues that lead you towards the conditional rendering approach?

@sdalonzo
Copy link
Collaborator

My immediate impression is that both are useful. We have some elements that are complex, DOM wise, that aren't ever used at mobile breakpoints, so I think there is some merit to not asking React to render and rerender those components on mobile devices that may be resource-constrained to begin with. For smaller cases, I think display: none is fine.

@zephraph
Copy link

We've been specifically dealing with the fallout of hydration failures when SSR markup doesn't match initial client renders. React's docs specifically calls it out as an error when the markup doesn't match. React 16 doesn't throw out the current tree and completely re-render, but tries to patch it up instead... it can lead to some challenges.

We're taking sort of a hybrid approach, but most everything relies on display: none. We have a wrapper component that we can flag certain breakpoints to do conditional rendering. It's a big process, but we're hoping to launch it to production soon. Once we do maybe we can pop back in and share our learnings.

@BeniCheni
Copy link
Collaborator

Ping / @sdalonzo , is this issue somewhat related to the Media Match component you developed recently? Perhaps we can relate and promote your version to DS to satisfy the requirement of this issue as well, in interest to to move forward?

@sdalonzo
Copy link
Collaborator

MediaQueryMatch is exactly what I was proposing here, @BeniCheni. The only blocker to porting it to DS is that I added breakpoints above and below the DS breakpoints to support our uses cases. Adding XXL would be easy enough, since it would just sit at the end of the array. But the smallest one would be a major breaking change because it would shift the whole array over.

@zephraph
Copy link

Hey folks! I forgot to follow up with this, but we recently published a blogpost on our experiences with responsiveness/react and some of the challenges we ran into (especially around server side rendering).

Feel free to check it out and shoot us questions if you have any: https://artsy.github.io/blog/2019/05/24/server-rendering-responsively/

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

No branches or pull requests

5 participants