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

Upstream #51

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Upstream #51

wants to merge 6 commits into from

Conversation

lwhorton
Copy link

@lwhorton lwhorton commented Sep 19, 2018

I'm not sure if this is how you would approach the problem, but it's what we came up with. See here for more details: #50.

TBH this is a really bad PR and should definitely not be merged. It works, and this was the bare minimum set of changes we had to make to get everything working without the env* context atom. A real refactor would be a much better solution, but we don't have the time.

- theres no need for an atom to track state if we just pass the class
through in a functional implementation
- turns out the env* atom was abused harder than we thought
- to be functional we have to pass the rule-index all around

... i see now why they used an atom.
- turns out the env* had a lot more hidden context
- fixed incorrect classnames being bandied about
Why:

* We want the ability to define separate media queries for a single
  element.

This change addresses the need by:

* Split up the media queries before building.
* Recombine separate media queries.
* Concat media queries.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants