Hi Erik,
>Why not commit the 2nd option for this fix to the 'develop' branch, ...
I've done the change (the 2nd option), and pushed it to the 'develop' branch.
Awaiting review and vote.
DarkStone
2014-12-18
At 2014-12-17 18:54:51, "Erik de Bruin" <e...@ixsoftware.nl> wrote:
>DarkStone,
>
>Why not commit the 2nd option for this fix to the 'develop' branch,
>and let Mustella run it's tests to see if nothing breaks? If someone
>objects, well, we "commit then review" and if someone vetoes the
>commit, we simply revert and have another look at the issue.
>
>Thanks,
>
>EdB
>
>
>
>On Wed, Dec 17, 2014 at 11:49 AM, DarkStone <darkst...@163.com> wrote:
>> Hi,
>>
>> I'm sorry, my bad, I missed a condition on that bug fix:
>>
>> This is wrong:
>> clearStyle("skinClass");
>>
>> This is correct:
>> styleProp == "styleName" ? clearStyle("skinClass") : null;
>>
>> But even with this change, there is still a bug if you do the followings:
>>
>> 1. Inside the constructor function of a SkinnableComponent subclass:
>> super();
>> setStyle("skinClass", SkinA);
>>
>> 2. In mxml, set the styleName="skinB" to that component.
>>
>> Run the application, SkinA shows, which is wrong, should be SkinB.
>>
>>
>> So, back to the start, right now, we still have to override the styleName
>> setter function in SkinnableComponent.as
>>
>>
>> That is, if you add
>>
>> styleProp == "styleName" ? clearStyle("skinClass") : null;
>>
>> to the styleChanged() function (line 552) of SkinnableComponent.as, there is
>> still a bug I described above.
>>
>>
>>
>> Or, if you just add
>>
>> override public function set styleName(value:Object):void
>> {
>> clearStyle("skinClass");
>> super.styleName = value;
>> }
>>
>> to SkinnableComponent.as, it will solve these bugs.
>>
>> I still prefer to the second one.
>>
>>
>> DarkStone
>> 2014-12-17
>>
>>
>> At 2014-12-17 18:05:43, "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
>>>>
>
>
>
>--
>Ix Multimedia Software
>
>Jan Luykenstraat 27
>3521 VB Utrecht
>
>T. 06-51952295
>I. www.ixsoftware.nl
