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
Merged

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 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

Loading

@gaearon
Copy link
Member

@gaearon gaearon commented Sep 4, 2018

Do you mind filing a separate issue with your proposal?

Loading

@gaearon gaearon merged commit 25d48a7 into facebook:master Sep 4, 2018
1 check passed
Loading
@mgol mgol deleted the grid-area-unitless branch Sep 4, 2018
@mgol
Copy link
Contributor Author

@mgol mgol commented Sep 5, 2018

@gaearon I submitted #13567 with the proposal.

Loading

@gaearon gaearon mentioned this pull request Sep 5, 2018
jetoneza added a commit to jetoneza/react that referenced this issue Jan 23, 2019
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

4 participants