MediaQuery.V2

[caseyWebb]

🔧 Modifying a component

Context

This PR is a major refactor/re-architecture of the MediaQuery module to use a builder pattern instead of exposing Css.Media.MediaQuery

Motivation

• Media queries are tricky to get right when you have multiple breakpoints. This is made worse by elm-css reversing the order of styles passed into it.
• The previous implementation was afflicted by a subtle discrepancy between mobile/notMobile styles with regards to screen readers and printers.
• In order to build efficient/minimal media queries and ensure the correct order, all media queries need to be known (and understood, i.e. "what is a breakpoint, so I can negate it if necessary") and styles output at once.

Also:

• Bumps Button, ClickableText, and Message. These functions expose an attribute constructor that used to expect a Css.Media.MediaQuery, they now accept Nri.Ui.MediaQuery.V2.MediaQuery.
• Adds hideTextForMobile to Button (was pending TODO for next major Button release)
• Refactors modules with doSomethingForMobile/doSomethingForNotMobile attributes to track MediaQuery.ResponsiveStyles and generate the media query at the last second to ensure auto-ordering benefits are realized.

Component completion checklist

• I've gone through the relevant sections of the Development Accessibility guide with the changes I made to this component in mind
• Changes are clearly documented
  • Component docs include a changelog
  • Any new exposed functions or properties have docs
• Changes extend to the Component Catalog
  • The Component Catalog is updated to use the newest version, if appropriate
  • The Component Catalog example version number is updated, if appropriate
  • Any new customizations are available from the Component Catalog
  • The component example still has:
    • an accurate preview
    • valid sample code
    • correct keyboard behavior
    • correct and comprehensive guidance around how to use the component
• Changes to the component are tested/the new version of the component is tested
• Component API follows standard patterns in noredink-ui
  • e.g., as a dev, I can conveniently add an nriDescription
  • and adding a new feature to the component will not require major API changes to the component
• If this is a new major version of the component, our team has stories created to upgrade all instances of the old component. Here are links to the stories:
  • add your story links here OR just write this is not a new major version
• Please assign the following reviewers:
  • Someone from your team who can review your PR in full and review requirements from your team's perspective.
  • Component library owner - Someone from this group will review your PR for accessibility and adherence to component library foundations.
  • If there are user-facing changes, a designer. (You may want to direct your designer to the deploy preview for easy review):
    • For writing-related component changes, add Stacey (staceyadams)
    • For quiz engine-related components, add Ravi (ravi-morbia)
    • For a11y-related changes to general components, add Ben (bendansby)
    • For general component-related changes or if you’re not sure about something, add the Design group (NoRedInk/design)

[bcardiff]

Is there a chance Tooltip.V3 will behave different (in a bad way) after these changes? Do we need to add a Tooltip.V4?

[caseyWebb]

There are the same potential impacts with regards to ::before { content: "..." }"/display: none`, but likely not being used anywhere. Still, not opposed to just versioning all of them as a matter of safety.

[bcardiff]

Why we need to do things slightly different here?

[caseyWebb]

This change is related to this change, which was made because withViewport was dropped.

As far as I could tell, that function is explicitly only being used as a workaround for the broken cascading. With that fixed, we can generate CSS for only the breakpoints we need, but we need to make sure that other properties up the cascade (that we aren't already overriding) are unset.

I'd prefer to not bring withViewport back, but if we didn't want to make these changes as drastic we could add another combinator to MediaQuery like MediaQuery.narrowMobile |> MediaQuery.and MediaQuery.not MediaQuery.mobile to target a specific viewport in a more generalized way.

[bcardiff]

This hints me to want to introduce new versions of all components that depends on MediaQuery.V2 since it's a change in behavior and it might be a blocker for upgrading in monolith if there is some regression.

[caseyWebb]

I'm not opposed 👍🏼 . The differences only apply in specific scenarios like ::before { content: "..."; } and display: none, but they are technically UX breaking changes–and very subtle ones at that.

[bcardiff]

Suggested change

	= MediaQuery Target (Maybe (List Style)) (Maybe (List Style))
	= MediaQuery Target { whenSatisfied : Maybe (List Style), whenUnsatisfied : Maybe (List Style) }

To reduce the changes of confusion of which is which

[caseyWebb]

good call I like this

[bcardiff]

Should we have a withAnimations = not prefersReducedMotion for convenience?

[caseyWebb]

probably for this one specifically since that's almost always how it will be used

[bcardiff]

Is there a benefit from one patter over the other? I would rather have a single api if possible

[caseyWebb]

Pipeline-style API is more flexible and lets us use phantom types for media query constructors–e.g. we can implement offset and only allow it to be used with breakpoints, since we can't offset a user preference.

The problem arises if you have 2 MediaQuery attribute with disparate attributes. Consider the following:

MediaQuery.init
  |> MediaQuery.on MediaQuery.mobile []
  |> MediaQuery.on MediaQuery.prefersReducedMotion []
  |> MediaQuery.toStyle

This works, because on interprets the type of the MediaQuery properties, does the right thing for that media query type, and returns a normalized ResponsiveStyles record. Since ResponsiveStyles is unbound, we can repeat this operation with a bunch of media queries without caring about the phantom types.

Now, consider the List-style API:

MediaQuery.fromList
  [ MediaQuery.mobile []
  , MediaQuery.prefersReducedMotion []
  ]

What is the type? MediaQuery { offsettable : () }? or MediaQuery {}.

Trick question, the answer is compiler error.

For end-consumers in practice since user preferences and breakpoints can't conflict in the cascade, this doesn't really matter. It will throw a potentially confusing compiler error, but can be resolved by using

[ MediaQuery.fromList [MediaQuery.mobile []]
, MediaQuery.fromList [MediaQuery.prefersReducedMotion []]

It is however an actual problem for any libraries that want to implement this. Consider this line in Tooltip that defines the responsive types on the config.

type alias Tooltip msg =
    { responsiveTooltipStyleOverrides : MediaQuery.ResponsiveStyles }

If you have this implemented/exposed strictly as a list-style API, you're in the same issue as above where you're restricted to media queries that share the same phantom type.

---

Pros of the list-style API:

• It covers most cases for general consumers that are applying the results directly to some HTML, not making re-usable components.
• It's more consise.
• It better matches existing Nri.Ui behavior*

*I have thoughts about this, given all of the above about the implications for phantom types and using combinators instead of a ton of permutations of attributes, and think broader adoption of this style api may actually be a good thing.