[ALOY-752] font property is not properly being mixed in when using multiple classes
GitHub Issue
n/a
Type
Bug
Priority
Medium
Status
Closed
Resolution
Fixed
Resolution Date
2014-04-15T19:03:19.000+0000
Affected Version/s
n/a
Fix Version/s
Alloy 1.4.0, 2014 Sprint 08
Components
Styling
Labels
qe-testadded
Reporter
Bert Grantges
Assignee
Tim Poulsen
Created
2013-07-18T20:57:01.000+0000
Updated
2014-10-28T15:48:37.000+0000
Description
When trying to use multiple classes that manipulate the font on say a Label, the font property, which is an object itself is not properly mixed in. Thus one class overwrites the other object as a whole.
Say you have the following example:
index.xml
index.tss
".h1":{
font:{
fontSize: "48dp"
}
},
".bold":{
font: {
fontWeight: "bold"
}
}
The expectation is that this should generate a label with a fontsize of 48 and in bold.
The actual result is that it will produce a bold label - but not at the correct size
Need to spend some time investigating if font should just get a specific fix, or deep merging of styles should be supported (always a dicey proposition in JS).
Bert Grantges 2013-07-29
Tony - i see that this was assigned to a sprint, what are you thinking about deep merging? One off for font, or extending to other styles?
Tony Lukasavage 2013-07-29
[~bgrantges] deep merging would allow me to never encounter this again, and cover all other possible objects, but I don't think I'm going to do that because of the inherent risks with attempting to do that in JS. I think I'll just more intelligently merge font where possible.
Malcolm Hollingsworth 2013-11-20
This will be a massive time saver and allow the proper use of the font object. It should be possible to use a mix and match approach here, the lack of this means much more complicated rules and styles.
I understand why this is such a pain - a historical feature that appears to have been stuck in the - with font being the only real object property.
May be an option would be to split them out in tss files only, so being able to specify as follows;
".h1": {
font: {
fontSize: "48dp"
}
},
".bold": {
fontWeight: "bold"
}
This would combine as;
font: {
fontSize: "48dp",
fontWeight: "bold"
}
The normal font object rules still apply and overwrite each other, but the use of a root level modifier property simply overrides that option. This would mean the preprocessor could perform a much simpler job without having to work through the object logic.
Again - this would be ONLY an Alloy tss feature, everywhere else this stays the same. This way it works more like the platform rule in its differentiation.
Just a thought.
Tim Poulsen 2014-03-14
PR https://github.com/appcelerator/alloy/pull/344
Functional test
1. Build the included test app /test/apps/testing/ALOY-752
2. The label should be large (30pt), bold, and in the Hoefler font, as specified by a mix of component, class, and ID selectors.
Malcolm Hollingsworth 2014-03-14
Still holding out for this one being resolved as the current situation makes it impossible to provide text styles in any simple manageable way.
Tim Poulsen 2014-04-15
PR tested and merged
Lokesh Choudhary 2014-05-09
Verified the fix. We see the label as expected & specified in the index.tss file.
Closing.
Environment:
Appc Studio : 3.3.0.201405011408
Ti SDK : 3.3.0.v20140508101423
Mac OSX : 10.8.5
Alloy : 1.4.0-dev
CLI - 3.3.0-dev
Mobileweb
Nexus 5 - android 4.4.2
Iphone 5 - iOS 6.1.3
Need to spend some time investigating if font should just get a specific fix, or deep merging of styles should be supported (always a dicey proposition in JS).
Tony - i see that this was assigned to a sprint, what are you thinking about deep merging? One off for font, or extending to other styles?
[~bgrantges] deep merging would allow me to never encounter this again, and cover all other possible objects, but I don't think I'm going to do that because of the inherent risks with attempting to do that in JS. I think I'll just more intelligently merge
fontwhere possible.This will be a massive time saver and allow the proper use of the font object. It should be possible to use a mix and match approach here, the lack of this means much more complicated rules and styles. I understand why this is such a pain - a historical feature that appears to have been stuck in the - with font being the only real object property. May be an option would be to split them out in tss files only, so being able to specify as follows; ".h1": { font: { fontSize: "48dp" } }, ".bold": { fontWeight: "bold" } This would combine as; font: { fontSize: "48dp", fontWeight: "bold" } The normal font object rules still apply and overwrite each other, but the use of a root level modifier property simply overrides that option. This would mean the preprocessor could perform a much simpler job without having to work through the object logic. Again - this would be ONLY an Alloy tss feature, everywhere else this stays the same. This way it works more like the platform rule in its differentiation. Just a thought.
PR https://github.com/appcelerator/alloy/pull/344 Functional test 1. Build the included test app /test/apps/testing/ALOY-752 2. The label should be large (30pt), bold, and in the Hoefler font, as specified by a mix of component, class, and ID selectors.
Still holding out for this one being resolved as the current situation makes it impossible to provide text styles in any simple manageable way.
PR tested and merged
Verified the fix. We see the label as expected & specified in the index.tss file. Closing. Environment: Appc Studio : 3.3.0.201405011408 Ti SDK : 3.3.0.v20140508101423 Mac OSX : 10.8.5 Alloy : 1.4.0-dev CLI - 3.3.0-dev Mobileweb Nexus 5 - android 4.4.2 Iphone 5 - iOS 6.1.3