API for 'adding' to generated class?
devdoomari opened this issue · 13 comments
is it possible to 'add' more to a generated classname?
e.g)
const someClass1Name = registerStyle({ ... });
...
freeStyle.addMoreTo( someClass1Name, {
'@media for iphone5 ', { ... }
);
freeStyle.addMoreTo( someClass1Name, {
'@media for ipad ', { ... }
);
(also, I need to have the order preserved...)
No. If you want this, you're probably using it wrong. If this existed, it would break the fundamental underlying property that every style is a hash of it's content. Could you explain what you want to achieve with this?
so
className1 = registerStyle( styleData )
returns:
className1 = someHash( styleData )
???
Then, how about extending registerStyle to have more-than-one inputs?
or 'registerStyles'?
className1 = registerStyles([
styleData1,
styleData1ForIphone,
styleData1ForIpad,
]);
Yes, the class name is computed from style, but I still don't understand what you'd like to see - can you give a concrete example?
it's all about having the order preserved for media queries...
So, for a bit more concrete example:
freeStyle.registerStyles([
{
width: 500
}, {
'@media for wider-than-iphone5 ': { width: 700 }
), {
'@media for wider than ipad': {width: 800, padding-right: 0}
}
]);
Ok, but that already occurs with the object form - do you have a concern that it's not working? It might be possible to make this change, so I'd welcome a PR. Do you have an idea of what we'd make it look like in the nested form?
Perhaps we go for a "tuple" style using an array of arrays where the the first item is the key and second is the style object? Gets tricky with the current approach though, because you insert the style object initially so there'd be no key.
but that already occurs with the object form
nope. the current object form can't preserve ordering - it's dictionary after all.
Perhaps we go for a "tuple" style using an array of arrays where the the first item is the key and second is the style object
That would be a real over-kill... of course you can do that by applying lodash function-dabs,
but it's not about height-comes-first-or-width-comes-first.
Those do not affect the rendered result. But ordering of CSS media queries DOES affect the end result.
@devdoomari Yes, it can. Have you even tested it? Please don't waste time arguing about something you can just check in a console. All browser engines have ordered by the way you wrote the object for a long time, and IIRC it's even formalised recently. For example:
That would be a real over-kill...
What are you talking about? I'm trying to help come up with a proposal like yours that works. I'm trying to work with you.
I'm still not sure what you're referring to when you suggest lodash function-dabs or the rest of that comment. If something I said didn't make sense, it would be great to let me know, because I'm really not sure where lodash comes into anything. All I was suggesting was a structure like:
registerStyle([
{ width: 500 },
['@media print', { color: red }],
])
The issue is it needs to be unambiguous. This works for a single level, but what about nesting?
registerStyle({
width: 500,
li: {
color: 'red',
'@media print': {
color: 'blue'
}
}
})
That needs to be supported with any syntax that gets adopted - we need to make sure it can work recursively.
I guess both approaches could use arrays of objects to indicate ordering. Then it comes down to whether we prefer the key to be part of a single-element object or separate as a tuple. This is definitely a viable alternate syntax we can introduce, but whatever proposal it is needs to be solid since it can never be undone. Also note that this may be non-trivial, since arrays today are 100% valid in value positions (
Line 89 in 8025335
so do you plan to do a fix on this?
I'm not keen on depending on Objects for preserving order
(it's funky on IE... / being on ES6 doesn't guarantee any browser compatibility except for chrome / FF )
-- maybe WeakMap (from ES6, usually with Babel-Polyfill) will do better for browser compatibility, as WeakMap shims are made with keeping order in mind.
Also, if you just remove sort tuples function inside FreeStyle, object-order is somewhat preserved...
(* but of course this will break a lot of test cases...)
Here's the test code:
const styleData: FreeStyle.Styles = {};
const style = FreeStyle.create();
styleData['color'] = 'red';
styleData['background-color'] = 'blue';
style.registerStyle(styleData);
const styleData2: FreeStyle.Styles = {};
const style2 = FreeStyle.create();
styleData2['background-color'] = 'blue';
styleData2['color'] = 'red';
style2.registerStyle(styleData2);
const style1str = style.getStyles();
const style2str = style2.getStyles();
console.error('style:', style1str);
console.error('style2:', style2str)
so do you plan to do a fix on this?
What do you expect to be fixed? So far, you haven't told me what is broken. Object order is already preserved. Can you please provide a reproducible case that is broken? Sorting CSS styles is a feature and has no impact on the output, but does make a big difference to hashing. I won't be changing how styles are ordered, and nested objects are already preserved in their correct order (they shouldn't be sorted, you can read the code).
Edit: Aren't -> shouldn't be, since they really shouldn't be but there could be an issue there I haven't encountered. I also agree that a syntax could be adopted to support arrays, we just need to agree on what it should look like and open a new issue about it. This issue was about "adding to a generated class" which seems different to what you're now asking for.