Hi Alex,
You are right, I tried using Type Selector to set default skinClass for the
custom SkinnableComponent, it worked.
In the constructor function of the custom SkinnableComponent
(test.TestComponent):
super();
//getQualifiedClassName(this) returns test::TestComponent
var fullClassName:String = getQualifiedClassName(this).replace(/::/g, ".");
//fullClassName is "test.TestComponent"
var css:CSSStyleDeclaration = new CSSStyleDeclaration();
css.setStyle("skinClass", skin.DefaultTestComponentSkin);
styleManager.setStyleDeclaration(fullClassName, css, true);
These codes above will solve all the problems we discussed before, it's a
little bit long, so here is the shortened one:
super();
var css:CSSStyleDeclaration = new CSSStyleDeclaration();
css.setStyle("skinClass", skin.DefaultTestComponentSkin);
styleManager.setStyleDeclaration(getQualifiedClassName(this).replace(/::/g,
"."), css, true);
So, I think we should do as you recommended in the last email, I will revert
the changes I made to SkinnableComponent on the 'develop' branch of Flex SDK
git, then we should update the Flex SkinnableComponent online help:
http://help.adobe.com/en_US/flex/using/WS460ee381960520ad-2811830c121e9107ecb-7ff9.html#WS460ee381960520ad-2811830c121e9107ecb-7ff7
And tell developers they should use Type Selector to set default skinClass for
their custom SkinnableComponent, and include the sample codes I provided above
(the shortened one).
DarkStone
2014-12-19
At 2014-12-18 05:02:52, "Alex Harui" <aha...@adobe.com> wrote:
>Hi Darkstone,
>
>I would prefer 1A) which is to revert the change, prove that Type
>Selectors work, and change the documentation for SkinnableComponent to
>guide people to use Type Selectors. Then overriding styleName should not
>be needed, and folks will build components following the same pattern as
>the rest of the SDK.
>
>Thoughts?
>-Alex
>
>On 12/17/14, 12:53 PM, "DarkStone" <darkst...@163.com> wrote:
>
>>Hi Alex,
>>
>>I got what you saying, I understand why you don't agree to the changes,
>>you've got your point, I think you are right.
>>
>>But that won't prevent others from doing it in the wrong ways.
>>
>>There is a paragraph in SkinnableComponent online help about implementing
>>the constructor:
>>
>>"Use the constructor to set the initial values of class properties. For
>>example, you can set default values for properties and styles, or
>>initialize data structures, such as Arrays. You can also set the
>>skinClass style to the name of your skin class."
>>http://help.adobe.com/en_US/flex/using/WS460ee381960520ad-2811830c121e9107
>>ecb-7ff9.html#WS460ee381960520ad-2811830c121e9107ecb-7ff7
>>
>>It encourages developers to write setStyle("skinClass", MyDefaultSkin) to
>>the constructor, if they want to set default skin for it.
>>
>>But doing so, set styleName on the MXML won't change the skin:
>><ns:MyComponent styleName="otherSkin"/>
>>
>>This will cause confusion, because built in Spark components like Button,
>>CheckBox, they all have their default skins, and if developers want to
>>use a different skin, they do this:
>><s:Button styleName="myButtonSkin"/>
>>
>>The confusion is:
>>
>><s:Button styleName="myButtonSkin"/> is working, the skin changes to
>>MyButtonSkin.
>>
>><ns:MyComponent styleName="otherSkin"/> is not working, the skin remains
>>default.
>>
>>They would be wondering why, and would assume this is a bug.
>>
>>So, the way I see it, there are two choices:
>>
>>1. Don't treat this as a bug, revert the changes I made, and update the
>>help docs about SkinnableComponent, tell developers they have to override
>>the styleName setter function by themselves, in order to make this case
>>work.
>>
>>2. Treat this as a bug, apply the changes I made.
>>
>>Both are OK for me, I still prefer the 2nd one, but I fully respect other
>>committers and PMC members' opinions.
>>
>>
>>DarkStone
>>2014-12-18
>>
>>
>>在 2014-12-18 03:28:49，"Alex Harui" <aha...@adobe.com> 写道：
>>>Hi Darkstone,
>>>
>>>Think of the general case. Let’s say that you also call:
>>>
>>>myComponent.setStyle(“fontWeight", “bold");
>>>
>>>myComponent.setStyle(“color", #ff0000);
>>>
>>>
>>>When you later call:
>>>
>>>myComponent.styleName = “otherSkin” and you have
>>>
>>>.otherskin {
>>> skinClass : MyOtherSkin;
>>> fontWeight : “normal”;
>>> color : #00FF00;
>>>}
>>>
>>>Why should skinClass be given special treatment? There is an order to
>>>the
>>>CSS lookup. Styles set on an instance override other styles specified in
>>>other selectors. I don’t think we should change that order, and I don’t
>>>think we should try to figure out what instance styles to clear. It
>>>isn’t
>>>really clear to me that folks will always want those styles cleared.
>>>
>>>It sounds like what your friend really wants is to specify a Type
>>>Selector
>>>for MyComponent and specify the default skin there instead of calling
>>>setStyle on each instance. Then styles set that have higher priority
>>>like
>>>styleName and instance styles will override with the intended result.
>>>
>>>CSS is expected to work a certain way. I don’t think we should change
>>>it,
>>>and I think your friend should be specifying a Type Selector.
>>>
>>>HTH,
>>>-Alex
>>>
>>>
>>>On 12/17/14, 10:06 AM, "DarkStone" <darkst...@163.com> wrote:
>>>
>>>>Hi Alex,
>>>>
>>>>Actually, this bug is reported to me by a Flex Developer, who is a
>>>>friend
>>>>of mine.
>>>>
>>>>The demand is, he wants to set a default skin class for his component
>>>>(which extends SkinnableComponent), so inside the constructor of his
>>>>component, he writes this:
>>>>setStyle("skinClass", MyDefaultSkin);
>>>>super();
>>>>
>>>>Then, he wants other people to apply a different skin if they want to,
>>>>other people would write this:
>>>><ns:MyComponent styleName="otherSkin"/>
>>>>
>>>>I think his demand is very common, now the problem is, by doing so, the
>>>>skin won't change.
>>>>
>>>>This bug only affects on custom SkinnableComponent, but it doesn't
>>>>affect
>>>>on any built in Flex Components like Spark Button, CheckBox, etc..
>>>>
>>>>That's why I think it is a bug : )
>>>>
>>>>
>>>>DarkStone
>>>>2014-12-18
>>>>
>>>>
>>>>At 2014-12-18 01:30:23, "Alex Harui" <aha...@adobe.com> wrote:
>>>>>Hi Darkstone,
>>>>>
>>>>>By this description, this is not a bug. Any style set via setStyle
>>>>>overrides styles in CSS selectors. The more common case is someone
>>>>>does
>>>>>want to always have myComponent use SkinA regardless of underlying
>>>>>changes
>>>>>to the selectors, like when style modules are loaded. If you want an
>>>>>overridden style to revert to using the styles of the selectors, then
>>>>>you
>>>>>have to call clearStyle in your code.
>>>>>
>>>>>-Alex
>>>>>
>>>>>
>>>>>On 12/17/14, 2:05 AM, "DarkStone" <darkst...@163.com> wrote:
>>>>>
>>>>>>Hi Alex,
>>>>>>
>>>>>>I've located the problem:
>>>>>>
>>>>>>Step 1. Using MySkinnableComponent.setStyle("skinClass", SkinA) to
>>>>>>apply
>>>>>>an initial skin is ok.
>>>>>>
>>>>>>Step 2. Then using MySkinnableComponent.styleName = "skinB" won't
>>>>>>work,
>>>>>>the skin won't change.
>>>>>>
>>>>>>When [Step 2] happens, inside the validateSkinChange() function of
>>>>>>SkinnableComponent.as (line 440-443):
>>>>>>
>>>>>>newSkinClass = getStyle("skinClass");
>>>>>>skipReload = newSkinClass && getQualifiedClassName(newSkinClass) ==
>>>>>>getQualifiedClassName(_skin);
>>>>>>
>>>>>>The getStyle("skinClass") returns the old skin (SkinA), which should
>>>>>>be
>>>>>>the new skin (SkinB).
>>>>>>
>>>>>>As a result, the newSkinClass && getQualifiedClassName(newSkinClass)
>>>>>>==
>>>>>>getQualifiedClassName(_skin) returns true, which should be false.
>>>>>>
>>>>>>That's why the SkinnableComponent won't change the skin.
>>>>>>
>>>>>>
>>>>>>
>>>>>>I know how to fix this now.
>>>>>>
>>>>>>No need to override the styleName setter function in
>>>>>>SkinnableComponent.as.
>>>>>>
>>>>>>Just add a line clearStyle("skinClass") to styleChanged() function in
>>>>>>SkinnableComponent.as (add at line 552):
>>>>>>
>>>>>>override public function styleChanged(styleProp:String):void
>>>>>>{
>>>>>> var allStyles:Boolean = styleProp == null || styleProp ==
>>>>>>"styleName";
>>>>>>
>>>>>> if (allStyles || styleProp == "skinClass" || styleProp ==
>>>>>>"skinFactory")
>>>>>> {
>>>>>> clearStyle("skinClass"); //This will fix the bug
>>>>>> skinChanged = true;
>>>>>> invalidateProperties();
>>>>>>
>>>>>> if (styleProp == "skinFactory")
>>>>>> skinFactoryStyleChanged = true;
>>>>>> }
>>>>>>
>>>>>> super.styleChanged(styleProp);
>>>>>>}
>>>>>>
>>>>>>clearStyle("skinClass"); //This will fix the bug
>>>>>>That's the only line needs to be added, in order to fix this bug.
>>>>>>
>>>>>>
>>>>>>I've already updated the JIRA for this
>>>>>>https://issues.apache.org/jira/browse/FLEX-34689
>>>>>>
>>>>>>
>>>>>>DarkStone
>>>>>>2014-12-17
>>>>>>
>>>>>>
>>>>>>在 2014-12-17 14:12:52，"Alex Harui" <aha...@adobe.com> 写道：
>>>>>>>
>>>>>>>
>>>>>>>On 12/16/14, 9:36 PM, "DarkStone" <darkst...@163.com> wrote:
>>>>>>>
>>>>>>>>Hi Alex,
>>>>>>>>
>>>>>>>>Why this needs to be fixed is simple:
>>>>>>>>
>>>>>>>>1. Using MySkinnableComponent.setStyle("skinClass", SkinA) to apply
>>>>>>>>an
>>>>>>>>initial skin is ok.
>>>>>>>>
>>>>>>>>2. Then using MySkinnableComponent.styleName = "skinB" won't work,
>>>>>>>>the
>>>>>>>>skin won't change.
>>>>>>>>
>>>>>>>>The styleChanged() function in SkinnableComponent doesn't handle
>>>>>>>>this
>>>>>>>>scenario correctly, that's why we need to override the styleName
>>>>>>>>setter
>>>>>>>>function to fix this.
>>>>>>>
>>>>>>>I’m wondering why the styleChanged() function can’t be changed to
>>>>>>>handle
>>>>>>>this scenario correctly. There are other ways to change skinClass,
>>>>>>>like
>>>>>>>setting it directly in a CSSStyleDeclaration or by loading a Style
>>>>>>>module.
>>>>>>> I would expect them all to call styleChanged() and the right thing
>>>>>>>to
>>>>>>>happen.
>>>>>>>
>>>>>>>Thanks,
>>>>>>>-Alex
>>>>>>>
>>>>>
>>>
>
