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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

[css-grid] move media query cell placements to come after cell styles #1057

Merged
merged 1 commit into from Jun 7, 2018

Conversation

@yepninja
Copy link
Collaborator

@yepninja yepninja commented Jun 7, 2018

Third solution without duplication media queries.

Now we have solutions for all 3 decisions that we discussed 馃槃

@ai
Copy link
Member

@ai ai commented Jun 7, 2018

Oh 馃槄

I prefer to duplicate medias. It is more stable for the cases (like 2-3 different media).

@yepninja
Copy link
Collaborator Author

@yepninja yepninja commented Jun 7, 2018

Here we decided to implement the solution without duplications.

@ai
Copy link
Member

@ai ai commented Jun 7, 2018

Since people afraid of Autoprefixer鈥檚 Grids, I think using safest way with a few extra media will be better to promotion.

They will be compressed by gzip and we need max safety.

cc @Dan503

@Dan503
Copy link
Contributor

@Dan503 Dan503 commented Jun 7, 2018

We agreed to do a half/half approach remember?

We don't want to shift author styles around in the declaration order. The goal is to create a single duplicate media query that holds all of the autoprefixer generated area code in it that is placed after the last grid area reference.

See my original example in the issue to see what I mean.

Creating a duplicate query for each area reference creates far too much code bloat and moving the full original media query is extremely dangerous.

@Dan503
Copy link
Contributor

@Dan503 Dan503 commented Jun 7, 2018

Option 2 is the method I'm talking about. That is the one we agreed to.

@ai
Copy link
Member

@ai ai commented Jun 7, 2018

@Dan503 but creating a single media for every area seems dangerous for me as well. Code bloat will be fixed by gzip. Are you sure that it is a good reason to not use the safest way?

@Dan503
Copy link
Contributor

@Dan503 Dan503 commented Jun 7, 2018

It's not every single one.

Take a good look at the example code I posted in the issue.

Notice that there is only one duplicate media query. There is not a duplicate for each declaration.

There is basically a duplicate query per media query per grid. It is not a duplicate query per query per area.

@ai
Copy link
Member

@ai ai commented Jun 7, 2018

There is basically a duplicate query per media query per grid. It is not a duplicate query per query per area.

Yeap I got it. And I am afraid that combining different areas (which could be in different place of CSS file) into a that second media query is not safe, since we move areas declaration to a different position.

@Dan503
Copy link
Contributor

@Dan503 Dan503 commented Jun 7, 2018

Yes I know, Autoprefixer can't handle grids sharing the same area names. It has that restriction hard boiled into it's area code which is why I opened issue #1038

I point this restriction out in the article and explain why it is the case so you won't need to worry about community backlash

@ai ai merged commit 34bad61 into master Jun 7, 2018
0 of 2 checks passed
0 of 2 checks passed
continuous-integration/travis-ci/pr The Travis CI build failed
Details
continuous-integration/travis-ci/push The Travis CI build failed
Details
@yepninja yepninja deleted the grid-media branch Jun 7, 2018
@ai
Copy link
Member

@ai ai commented Jun 7, 2018

Released in 8.6.1

@Dan503
Copy link
Contributor

@Dan503 Dan503 commented Jun 7, 2018

I'll do the article updates tomorrow. Thanks for getting this over the line :D

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants