Update Widget.getSkinName() to allow multiple skin prefixes

[aversini]

WF uses YUI3 to build its own framework. We are using most of YUI3 widgets but we also need to have our developers working on custom WF widgets. And to make things clear, we need to prefix our widgets skin differently than the YUI ones.

For example, we could use yui3-skin-night on a page, but we may also need to have to that same page a custom WF widget with the skin wf2-skin-toad.

Since it's only possible to change the 'yui3' prefix globally with the configuration, we took the approach of extending the capability of getSkinName: if a prefix is given, the behavior is the same as before except that the search will be made on 'prefix-skin' instead of the default 'yui3-skin'. Of course, if no prefix is given, the default remains 'yui3-skin' which ensure backward compatibility with YUI3.

Let me know if that's something that has any interest to you guys!
Thanks a lot!
Arno

[ryanvanoss]

+1

[sdesai]

Thanks. Looks like it's worth pulling in. Not sure if the Alloy UI folks have a similar need also ( @eduardolundgren ? )

[jconniff]

The work I'm doing with skin generating could accommodate a user defined prefix. The result would be the ability to generate a set of skins that would style all the YUI Library widgets having a different prefix such as 'wf2-skin-toad'. I wonder if that's worth doing, or is the desire more for custom widgets? The YUI Library widgets would have to generate HTML markup containing matching 'wf2-skin-_____' class names.

[aversini]

Yes, supporting a CSS prefix different than the Y.config one is the core of the request.

I did think about overriding the delimiter as well since they are pretty tied together in Y.config, but ruled against it as it would imply updating getSkinName() and getClassName(). Furthermore, there is a flag in getClassName to ignore the prefix altogether but there is no special treatment for the delimiters...

That said, I agree it would make it more generic. I'm going to look into updating the code to include custom delimiters as well, with as little impact as possible!

[aversini]

Hi guys,

After looking more deeply it seems to me that adding the possibility to modify the default delimiter is too risky a change for now. I'd rather have the original pull request focused only on the CSS prefix, and then make another pull request later to include the delimiter as well... For now, I really think we should just limit changes to the bare minimum to make sure backward compatibility is preserved.

Let me know what you think! Thanks!

[sdesai]

As long as we're leaving a forward compatible path open to support the delimiter in the future (as far as the getSkinName signature goes) - which it seems like we do - that seems reasonable to me.

I was really thinking this would just be another parameter to getSkinName(prefix, delim).

That is, if the end user wants to change things for the whole instance, they use the config for both prefix and delimiter and they don't need to use these arguments for getSkinName.

If they don't want to change things globally, but want to support a skin prefix like foobar_aqua or foobar_blue, or foobar_skin_aqua, foobar_skin_blue they can use the parameters to getSkinName, while the generated classes would still be yui3-widget-foo etc.

It just seemed arbitrary that in the implementation code, we were using the arg for one part of the customization, but the configured config value for another.

[aversini]

Yes, I think the path is open to support the delimiter in the future, and add an extra parameter to getSkinName.

The reason why it's simple to modify the prefix but not the delimiter at the implementation code is rooted in the way Y.ClassNameManager.getClassName works. It accepts an extra flag to ignore the configured prefix in the search - so when we add our own prefix at the getSkinName level, we rely on that particular flag to ignore the configured prefix...

Adding an extra arg to getSkinName to take care of the delimiter is not as simple as switching this flag anymore but we would have to rework Y.ClassNameManager.getClassName as well...

[aversini]

I've updated a little bit my changes to take into account all the feedback I received:

• Removed the dependency on classmanager
• Adding the option to specify a custom delimiter (as well as a custom prefix)

Let me know if that looks good! Thanks!

[sdesai]

Thanks. I'll pull it in

[sdesai]

Hey Arno, so I took a look at the new implementation, and felt that not going through ClassNameManager for the 99% default use case was not appropriate.

So, after messing with a couple of options, I settled on this which seems like it addresses your use case (and more custom options, with delimiters/prefixes etc specifically for the skin prefix - which we discussed at YUIConf), without compromising the 99% use case.

sdesai@840e38b

Specifically, getSkinName():
https://github.com/sdesai/yui3/blob/840e38b0981e1f7cbcca08e6909a2a5f56a0196c/src/widget/js/WidgetSkin.js#L36

If you're OK with the above impl, I'll close out this pull request and issue a new one based on the above branch. Let me know.

[aversini]

Hey Satyen, sorry for the long holiday break :)
Yes, your last solution looks great and totally satisfies our needs!

[sdesai]

Thx. I'll issue a new request.