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

Uses "useResponsiveQuery" hook over "withPlaceholder" in Only component #310

Merged
merged 3 commits into from
Apr 1, 2020

Conversation

kettanaito
Copy link
Owner

@kettanaito kettanaito commented Mar 31, 2020

Changes

  • Uses "useResponsiveQuery" hook over "withPlaceholder" in Only component
  • Adds an E2E test that asserts that Only's children preserve state when the Only's parent updates

Issues

Release version

  • internal (no release required)
  • patch (bug fixes)
  • minor (backward-compatible changes)
  • major (backward-incompatible, breaking changes)

Contributor's checklist

  • My branch is up-to-date with the latest master
  • I ran yarn verify to see the build and tests pass

@kettanaito
Copy link
Owner Author

Hi, @Vidlec. Could you please get a look at this pull request? It should fix the issue you are experiencing. Thank you!

Copy link
Contributor

@Vidlec Vidlec left a comment

Choose a reason for hiding this comment

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

This looks amazing!

Vidlec
Vidlec previously approved these changes Apr 1, 2020
// Render always when no constrains are provided
return <>children</>
const matches = useResponsiveQuery({ for: exactBreakpoint, from, to, except })
return matches && <Box {...restProps}>{children}</Box>
Copy link
Owner Author

Choose a reason for hiding this comment

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

Instead of rendering in a fragment and deligating the restProps assignment to the withPlaceholder (MediaQuery, effectively), the Only component now renders a Box, to which it assigns all the passed box props. This should not introduce any breaking changes, as it's a matter of moving the abstracted parent component up to the application surface.

@kettanaito kettanaito merged commit fabc2f4 into master Apr 1, 2020
@kettanaito kettanaito deleted the 308-only-rerender branch April 1, 2020 13:59
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.

Rerendering of parent component causes Only to unmount and mount its children
2 participants