React hooks - add useMedia

[sylvhama]

Description

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.

I've adapted the code from https://usehooks.com/useMedia/

• 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]

Why does the linter does not like having an empty dependency array? What if I want to simulate componentDidMount and componentWillUnmount?

[loic-d]

You have to disabled the rule with:
// eslint-disable-next-line react-hooks/exhaustive-deps

[loic-d]

We should return a default value (false?) if you are rendering on the server.

 if (typeof window === 'undefined') {
    return false;
  }

[qq99]

false.addListener(...) below will 💥

[loic-d]

Sorry I missed that. You can return a noop object matching the signature of what you would expect on the client:

const noopMediaQueryList = {
  media: '',
  onchange: noop,
  addListener: noop,
  addEventListener: noop,
  removeListener: noop,
  removeEventListener: noop,
  dispatchEvent: () => false,
  matches: false,
};

[sylvhama]

Thanks I totally forgot about SSR!

[qq99]

Related potential https://github.com/Shopify/web/pull/14306

What are your thoughts on the hook that only handles a single media at a time?

const isSmallScreen = useMediaQuery('(max-width: 640px)');
const isMediumScreen = useMediaQuery('(max-width: 720px)');

I feel like it gives you a nicer handle (semantically) on the thing, if you wanted to do something like:

if (isSmallScreen) {
  return <SmallVersion/>
} else if (isMediumScreen) {
  return <MediumVersion/>
} else {
  ....
}

with this PR's hook, I think you lose a bit of information because it condenses all the info into a single message result variable. To achieve the same, you might have:

  const screenSize = useMedia(
    ['(max-width: 640px)', '(max-width: 748px)'],
    ['small', 'medium'],
    'default',
  );

  if (screenSize === 'medium') { ... }
  /// etc

My version of the hook also decouples what you're querying from your eventual markup, and is imo simpler because it's dealing with a single media query

---

At the time, I wrote that you might lose information, so I'm not certain (depending on the order you declared your values array) whether you'd get a sensible result from the useMedia hook if you needed that kind of information

e.g.,

  const screenSize = useMedia(
    ['(max-width: 748px)', '(max-width: 640px)'],
    ['medium', 'small'],
    'default',
  );

would this return screenSize === 'small' for 300px?

vs.

  const screenSize = useMedia(
    ['(max-width: 640px)', '(max-width: 748px)'],
    ['small', 'medium'],
    'default',
  );

for 300px, would this return screenSize==='medium'?

They both match, so I read it as the last to match is the value you get returned. My version of the hook has no order of declaration issues

[sylvhama]

@qq99 for sure yours is muuuch simpler to use / understand. You're right that passing arrays is very confusing. Plus we can achieve the same goals with yours. Should we re-open your PR in quilt?

[qq99]

That sounds good to me @sylvhama