Skip to content

Add CSS Grid test for old & new syntaxes - #2189

Merged
KuraFire merged 3 commits into
masterfrom
feature/css-grid
Apr 13, 2017
Merged

Add CSS Grid test for old & new syntaxes#2189
KuraFire merged 3 commits into
masterfrom
feature/css-grid

Conversation

@KuraFire

Copy link
Copy Markdown
Member

While CSS Grid gets rolled out, it’s useful to have a way to target
IE11 which does not support @supports() but does support old CSS Grid
syntax, as does the new IE14.

While CSS Grid gets rolled out, it’s useful to have a way to target
IE11 which does not support @supports() but does support old CSS Grid
syntax, as does the new IE14.
Probably better not to make builds fail.
@ryanseddon

Copy link
Copy Markdown
Member

Can you add this to config-all.json too please

@ryanseddon

Copy link
Copy Markdown
Member

Normally we have each test separate but I think this makes sense to put it in one since most people will want both.

Comment thread feature-detects/css/cssgrid.js Outdated
"name": "The _old_ CSS Grid",
"href": "https://www.w3.org/TR/2011/WD-css3-grid-layout-20110407/"
}],
"warnings": ["`grid-columns` is only in the old syntax, `grid-column` exists in both and so `grid-template-rows` is used for the new syntax."]

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this isn't really a warning. Warnings should be about the detect themselves (i.e. 🚨 giamundo test🚨 , or listing possible edge case failures. Since the end user of modernizr won't need this information, but modernizr developers would, could you move this to a straight up JS comment on line 17?

- added new feature detect to lib/config-all.json
- made the notice a JS comment instead of a formal warning
@patrickkettner

Copy link
Copy Markdown
Member

lgtm

@KuraFire
KuraFire merged commit 6ee39b9 into master Apr 13, 2017
@Stexxen Stexxen mentioned this pull request Apr 14, 2017
@ryanseddon
ryanseddon deleted the feature/css-grid branch April 17, 2017 23:52
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.

3 participants