Skip to content
This repository has been archived by the owner on Apr 7, 2020. It is now read-only.

[RFC] Screen Size Context Refactor Proposal #299

Open
brad-decker opened this issue Jun 15, 2018 · 4 comments
Open

[RFC] Screen Size Context Refactor Proposal #299

brad-decker opened this issue Jun 15, 2018 · 4 comments

Comments

@brad-decker
Copy link
Contributor

First a little background into why I think a refactor is appropriate, and necessary, then i'll go into the details of the implementation plan.

  1. By making the distinction of what screen size keys to support a userland configurable item it does make this platform more mutable, but it also doesn't let us enforce a strict material spec as we become more compliant to spec.
  2. For the same reason mentioned above, it prevents us from being able to afford more power to the end user. For example, we could move the 'isMobile' boolean into screenSizeContext consumer, and add in the observable bits api to allow for a end user to only subscribe to changes in that field.
  3. I can refactor the column/grid/rows to use media queries and pure flex instead of being so heavily dependent on application state. I think this is the correct way to implement a flex grid and would cut out a lot of overhead in our responsiveness.

Implementation Details for Round 1

  1. We migrate the screensizes from merlin into the smc code base and hardcode those breakpoints in, and we guarantee that the keys of xs->xl are defined at the SMC level.
  2. Bring the isMobile helper into SMC, flesh out a little bit and maybe a 'mode' key of 'mobile, tablet, desktop' is more appropriate.

^- although a big change this wouldn't require a significant refactor of our code base, and would allow future changes to be incremental.

Implementation Details for Round 2

Not fully fleshed out right now, but the columns and rows can be made much more performant with simple media queries instead of config objects that must tap into state and re-render on all state updates. This would be the huge impact to the code base cause the API would have to change to support this. The API would be simpler, but we need to change everything over.

@brad-decker
Copy link
Contributor Author

RichardM [4 minutes ago]
but it also doesn't let us enforce a strict material spec as we become more compliant to spec.
could you please explain this? (edited)

@richTheCreator answering here to keep a permanent record of conversation for public benefit.

By allowing the end user to declare their own breakpoints we have to support a much wider set of features. Right now there is no guarantee the end user will use our keys, for example. So columns and rows have to accept props for whatever keys the user defines for their breakpoints. It also means we limit our ability to implement components in strict compliance with material design because the end user can mutate the media queries. So if material spec says a button should have a specific gutter at a specific resolution we can put those defaults in but we introduce the ability for a poor user (developer) experience when they go to customize the breakpoints.

I think i'm in favor of tightening the scope of the theme and what we really allow users to change.

@richTheCreator
Copy link

richTheCreator commented Jun 15, 2018

Thanks for the explanation @brad-decker. Makes sense! Excited to see how this would lower the overhead for responsiveness out of the box.

@AriLFrankel
Copy link
Contributor

Regarding screenSizeConsumer, I am a fan of the idea of using material compliant breakpoints for all three of the reasons @brad-decker mentioned. I totally support enforcing material standards, and I think that the more powerful API that this allows us to expose is a huge win. Also great to make the grid more performant by being able to do a pure CSS implementation.

Regarding withScreenSize and our current responsive strategy, I did an audit of our usage of withScreenSize. Definitely possible that I'm missing some use cases, but it seems like our use cases across our org's code fall into two broad categories:

Applying mobile styles intended for mobile screen size and/or conditionally displaying elements

For mobile styles, Those are a light lift to reword as media queries
For conditionally displaying elements on mobile screens, those can use this pattern instead, using display in combination with media queries
I like @brad-decker's idea of exposing material compliant breakpoints on SMC in some way. I think mobile, tablet, and desktop media queries or as a styles mixin could be a cool way to do that.

Functionality that is platform specific, not screen size specific

Since these are intended to relate to platform and not screen size, it seems like these could be well served by a withPlatform enhancer separate from withScreenSize. We could break out the platform.js logic into its own HOC.

see here for examples of these patterns

@iamdavid0091
Copy link
Contributor

👍

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

No branches or pull requests

4 participants