[jira] [Commented] (PIVOT-1014) Specify skin default styles in JSON file so they can be easily changed

["Roger Whitcomb (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/PIVOT-1014?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16273205#comment-16273205 ] 

Roger Whitcomb commented on PIVOT-1014:
---------------------------------------

First POC submission made, with the framework and two classes now using it:
Sending        core\src\org\apache\pivot\beans\BXMLSerializer.java
Sending        core\src\org\apache\pivot\beans\BeanAdapter.java
Sending        core\src\org\apache\pivot\collections\Dictionary.java
Sending        core\src\org\apache\pivot\json\JSONSerializer.java
Sending        wtk\src\org\apache\pivot\wtk\MessageType.java
Sending        wtk\src\org\apache\pivot\wtk\Theme.java
Sending        wtk-terra\src\org\apache\pivot\wtk\skin\terra\TerraExpanderSkin.java
Sending        wtk-terra\src\org\apache\pivot\wtk\skin\terra\TerraRollupSkin.java
Sending        wtk-terra\src\org\apache\pivot\wtk\skin\terra\TerraTheme.java
Adding         wtk-terra\src\org\apache\pivot\wtk\skin\terra\terra_theme_defaults.json
Transmitting file data ..........done
Committing transaction...
Committed revision 1816747.

PIVOT-1014:  Create the starting framework for loading default styles
from a JSON file.  Do a couple of Terra*Skin classes as a test.
1. In the Theme interface, add a new interface method: "setDefaultStyles".
2. In TerraTheme, add the loading of the new "terra_theme_defaults.json"
   file as part of the constructor, saving the loaded map.  Then implement
   the "setDefaultStyles" method using the initially loaded map.
3. To help with debugging problems of missing setter methods, add the key
   or property name being set to the "BeanAdapter.coerce(...)"" method and
   add this new parameter to all the callers.
4. Also add a default "putAll" method to Dictionary, and slightly modify
   the parameters of the existing method of this name in BeanAdapter so
   it can override this new default method.
5. Move the style initialization in both TerraExpanderSkin and
   TerraRollupSkin into the new .json file.  This involves several other
   changes:
   a. The style initialization has to be moved into the "install()" method
      in order for the component to be set (which is needed for the "getStyles()"
      call in there).
   b. A number of new "set*Color(int)" methods had to be added for
      TerraExpanderSkin because they were now being invoked to set the defaults
      but were missing (and thus the need for identifying the property name in
      the "BeanAdapter.coerce()"" method).
   c. Also remove some of the now unnecessary derived color calculations in these
      two constructors, since they are done in the setter methods.
6. Do some other minor code simplification in TerraTheme, which also involved:
   a. Make a new "fromString(...)" method in "MessageType" to hide some of the
      repeated work being done in TerraTheme to load the icons.

Note: this is more-or-less a Proof-Of-Concept for this issue, which seems to be
reasonable, and fulfilling my objectives for it.  There are still 51 other classes
to be done/changed, and the slowdown implications have not yet been measured.


> Specify skin default styles in JSON file so they can be easily changed
> ----------------------------------------------------------------------
>
>                 Key: PIVOT-1014
>                 URL: https://issues.apache.org/jira/browse/PIVOT-1014
>             Project: Pivot
>          Issue Type: Improvement
>          Components: wtk-terra
>            Reporter: Roger Whitcomb
>            Assignee: Roger Whitcomb
>            Priority: Minor
>             Fix For: 2.1
>
>
> I have noticed some inconsistencies in the way default styles (things like colors, fonts, padding/margins, etc.) are set in the constructors of the Terra*Skin classes, such as:
> * Color values set directly from the theme vs. set through the setter methods.
> * Missing color setter methods using the theme color index.
> * Derived colors being set in two different places, potentially leading to inconsistencies because of the duplication.
> * Possible inconsistencies between controls in the theme colors used (such as for backgrounds, borders, etc.) -- this has not been verified, because it will require a lot of research.
> * Since these defaults are all set in code, even though (for instance) the theme colors themselves are set in the "TerraTheme_default.json" file, changing the overall look-and-feel is difficult at present.
> So, for all these reasons I propose to use a new "terra_theme_defaults.json" file, which can be overridden by a new setting in the "TerraTheme*.json" file(s) which will set all the defaults for all the Terra*Skin classes similar to this (taken from TerraExpanderSkin):
> Previously:
> {code:java}
>        setBackgroundColor(theme.getColor(4));
>        titleBarBackgroundColor = theme.getColor(10);
>        titleBarBorderColor = theme.getColor(7);
>        titleBarColor = theme.getColor(12);
>        shadeButtonColor = theme.getColor(12);
>        disabledShadeButtonColor = theme.getColor(7);
>        borderColor = theme.getColor(7);
>        padding = new Insets(4);
> {code}
> New:
> {code:java}
>     TerraExpanderSkin : {
>         backgroundColor : 4,
>         titleBarBackgroundColor : 10,
>         titleBarBorderColor : 7,
>         titleBarColor : 12,
>         shadeButtonColor : 12,
>         disabledShadeButtonColor : 7,
>         borderColor : 7,
>         padding : 4
>     },
> {code}
> Note: this approach also fits well with the overall Pivot philosophy of doing things in a "declarative" fashion (via BXML / JSON files) rather than in code, where possible.
> I have prototyped this in a couple of classes and it seems to work well, AND it has revealed a number of the color properties that were missing the "setXXX(int index)" methods.
> Note: there is potential for slowdown during control construction since we're executing a LOT more code to set these defaults (including expensive reflection operations inside BeanAdapter), but I have not yet been able to measure the speed differential to see if it will be of concern.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)