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

Shorthand props to longform question #3

Closed
pwlmaciejewski opened this issue Oct 19, 2016 · 1 comment
Closed

Shorthand props to longform question #3

pwlmaciejewski opened this issue Oct 19, 2016 · 1 comment

Comments

@pwlmaciejewski
Copy link

Hi. I was wondering if adding a new feature of auto-rewriting shorthand properties into long-form would make sense for this project? Here's a example:

input:

   padding: 0

output object:

{
    paddingLeft: 0,
    paddingRight: 0,
    paddingTop: 0,
    paddingBottom: 0
}

Here's why. There's this bug here in FormidableLabs/radium#95 which leads further to this discussion facebook/react#6348. The problem (as I understand it) is that mixing short-hand and long-form props in one object leads to weird results. In recent versions of Radium you get a warning when you mix them. Solution for now is to avoid short-hand props where there are conflicts but if react-styling could expand those then that would be a nice solution for the problem maybe?

@catamphetamine
Copy link
Owner

Yes, I've had this bug before and didn't do anything about it, but such smart autofixing would really do the job.
This library could expand all properties, but would all people like it this way?
Anyway, I guess I would merge a Pull Request implementing such a feature.
If you have time (currently I don't) then you can implement this autoexpander.
It has to reside in a separate file and must be covered with basic expansion tests.

Daniil Abramov talks about a thing called style-builder and seems that it already handles expansion (and has the tests).
So it can be included into this library as a dependency.

Where can such expansion be performed?
I'd suggest somewhere here maybe:
https://github.com/halt-hammerzeit/react-styling/blob/master/source/index.js#L41

    // parse text into JSON object
    const style_json = parse_style_class(tabulator.extract_tabulation(lines), [])

    // expand "modifier" style classes
    return expand_modifier_style_classes(style_json)
}

It could be style_json = Style_builder.build(...)

Expansion tests would be required in this case (e.g. tests/expansion.js).

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

No branches or pull requests

2 participants