Skip to content
This repository has been archived by the owner on Oct 1, 2024. It is now read-only.

React hooks - add useMedia #1364

Merged
merged 3 commits into from
Apr 14, 2020
Merged

React hooks - add useMedia #1364

merged 3 commits into from
Apr 14, 2020

Conversation

sylvhama
Copy link
Contributor

@sylvhama sylvhama commented Apr 9, 2020

Description

This is a new attempt to add a useMedia hook after #1338. This one is inspired from https://github.com/Shopify/web/pull/14306, the code was first made by @qq99 <3

When using a design system such as Polaris, you might want to change props values based on a media query instead of doing custom css that could break if the design system changes its implementation. E.g. Imagine I might want to hide a TextField label via its labelHidden prop. This is not meant to replace CSS media queries, it's really meant for specific scenarios.

Type of change

  • Patch: Bug/ Documentation fix (non-breaking change which fixes an issue or adds documentation)
  • react-hooks Minor: New feature (non-breaking change which adds functionality)
  • Major: Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist

  • I have added a changelog entry, prefixed by the type of change noted above

@sylvhama sylvhama force-pushed the react-hooks/useMedia branch from fe728d1 to 2625d53 Compare April 9, 2020 18:15
@sylvhama sylvhama force-pushed the react-hooks/useMedia branch from 2625d53 to 824bc9d Compare April 9, 2020 18:41
Copy link
Contributor

@qq99 qq99 left a comment

Choose a reason for hiding this comment

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

As I wrote the original code, you should get 2 other reviewers who are not me :)

@sylvhama
Copy link
Contributor Author

If you want to tophat it, you can use yarn tophat react-hooks ../web and test https://github.com/Shopify/web/pull/25763

Copy link
Contributor

@attila-berki attila-berki left a comment

Choose a reason for hiding this comment

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

LGTM

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

Successfully merging this pull request may close these issues.

4 participants