hcsuk commented on issue #341: Adding listener for when a bindable style has 
changed
URL: https://github.com/apache/royale-asjs/pull/341#issuecomment-437780922
 
 
   Yes I know what you mean re. adding too much in base classes.. 
   
   Just on the `SimpleCSSValuesImpl` vs `AllCSSValuesImpl` case: there aren't 
actually any significant differences between these classes; lots of minor diffs 
which are due to people updating one class with small changes but not the other 
(mostly updating the 'simple' one e.g. `if (o)` had been changed to `if (o !== 
undefined)` etc. Then the only real functional changes are:
   
   1. AllCSSValuesImpl has two additional properties being set on a 
'conditionalCombiners' object
   2. AllCSSValuesImpl would avoid putting "px" on the end of a fontWeight style
   3. SimpleCSSValuesImpl handles style strings passed in as "some-thing" and 
changes them to "someThing"
   4. SimpleCSSValuesImpl has an additional code block to implement `addRule` 
in JavaScript.
   
   So I can't really see a good reason to not just combine these two; the 
additions that are in 'simple' I definitely think should be made in 'all' as 
well.. I guess we could have those extra changes from 'all' kept out of 
'simple', but I would have thought the argument for that was slim.
   
   From the description, I had expected there to be a whole extra set of styles 
that the 'all' class supported, but it looks to me like these classes don't 
really have so much of the knowledge of the actual styles - unless it's around 
the various categories i.e. which ones are colors, which are numbers, etc.  The 
initial commit comment suggests the creation of the 'all' class was to slim 
down the simple case, so perhaps we still want the 'all' class which would be 
where future style categories could be added; but 99% of the functionality 
would still be common so inheriting this from the 'simple' case seems a good 
idea to me..
   
   
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

Reply via email to