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

Add gridArea to unitless CSS properties #13550

Merged
merged 1 commit into from
Sep 4, 2018

Conversation

mgol
Copy link
Contributor

@mgol mgol commented Sep 4, 2018

Ref #9185

Based on jquery/jquery@f997241

grid-area accepts a numeric value which then translates to grid-row-start, setting grid-row-end, grid-column-start & grid-column-end to auto.

I thought about adding a test but I don't understand how the ones in CSSPropertyOperations-test.js are supposed to work. For example, setting grid-row to 10 will result in a normalized gridRow style property with a value "10 / auto". It seems they work just because they use ms-prefixed properties in format not actually used in IE as IE's grid properties were named differently?

Another remark - in jQuery we have been auto-appending px to most numerical values as well but we decided it was a bad decision. It doesn't scale as it requires us to add more & more properties to the list. We've actually exposed the list at jQuery.cssNumber so that people don't always have to wait for us to add support for a property and do a release. What's more confusing, some of them would work both with & without the px suffix and that changes the meaning.

That's why we decided that in jQuery 4 we'll drop the auto-prefixing blacklist and turn to a whitelist that lists only a few most common properties to which we want to auto-append px (mostly because they're extremely common and we don't want to break the world too much); we plan to not expand that list. You can see the current plan in my PR: jquery/jquery#4055. In particular, see the proposed whitelist in a (visualized) regexp in:
https://github.com/jquery/jquery/blob/03e9dba3882868e1ee79f1fb0504326da925644f/src/css/isAutoPx.js.
Something to consider for a future React version.

@pull-bot
Copy link

pull-bot commented Sep 4, 2018

Details of bundled changes.

Comparing: 877f8bc...c4f1d23

schedule

File Filesize Diff Gzip Diff Prev Size Current Size Prev Gzip Current Gzip ENV
schedule.development.js n/a n/a 0 B 19.17 KB 0 B 5.74 KB UMD_DEV
schedule.production.min.js n/a n/a 0 B 3.16 KB 0 B 1.53 KB UMD_PROD

Generated by 🚫 dangerJS

@gaearon
Copy link
Collaborator

gaearon commented Sep 4, 2018

Do you mind filing a separate issue with your proposal?

@mgol
Copy link
Contributor Author

mgol commented Sep 5, 2018

@gaearon I submitted #13567 with the proposal.

@gaearon gaearon mentioned this pull request Sep 5, 2018
jetoneza pushed a commit to jetoneza/react that referenced this pull request Jan 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants