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-2811830c121e9107ecb-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 >>>>> >>> >
