royale-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From GitBox <...@apache.org>
Subject [GitHub] hcsuk commented on issue #341: Adding listener for when a bindable style has changed
Date Mon, 12 Nov 2018 07:20:21 GMT
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:
users@infra.apache.org


With regards,
Apache Git Services

Mime
View raw message