Hi Carlos,

That's why we are using Harb's solution with his own class list. If you
went through this long thread carefully I have an issue which end up with
that solution. The problem was exactly as Harb's mention. Try it yourself.

Another great things which Harb's bring up here is why I also like Royale.

1) I had really hard time to debug things when something was in CSS file
only not as inline in some Bead or Code. - That was really pain and I had
quite short css file. Imagine having 1000 lines in such file ?
2) I saw your commit and one question which come up to my mind. If you have
following line:

child.className += " layout vertical";

You hit in your IDE CTRL/CMD and try to click on " layout vertical" - does
it jump to the proper css classes in file ?

If not - That is another disadvantage of that approach - How do I find my
"layout vertical" class among 1000 lines and many css files in the project ?

I really really don't want that our framework will turns into an monolithic
AngularJS. Just because the other are doing things in that way we don't
have to follow, because we turn things in a mess.

I will shot you some thoughts to the code on GitHub.

Thanks, Piotr


2018-03-12 17:21 GMT+01:00 Harbs <harbs.li...@gmail.com>:

> I’m pretty sure your solution will only work if the user doesn’t set a
> className of their own. Setting className overwrites the entire classList.
>
> > On Mar 12, 2018, at 5:45 PM, Carlos Rovira <carlosrov...@apache.org>
> wrote:
> >
> > Hi
> >
> > I made some simplification that works ok in Jewel:
> >
> > 1.- remove CSSClassList and use element.classList since is native and
> > supported in all browsers we target, this simplifies code, and removes
> > classes from core.
> > 2.- I still need to use some additional code that can be simplified. I'm
> > doing:
> >
> > element.classList.toggle("primary", value);
> > setClassName(computeFinalClassNames());
> > classList has its own toggle function that makes super easy to manage
> adds
> > and removes, so no need to have a custom function in royale
> >
> > that uses:
> >
> > COMPILE::JS
> > override protected function computeFinalClassNames():String
> > {
> > return super.computeFinalClassNames() + " " + element.classList;
> > }
> >
> > I'd like to remove that and change the "setClassName" call to nothing, if
> > we change UIBase to simple use classList
> >
> > My guess is that we can have "typenames" and "classNames" but once all
> set,
> > all can be managed with classList to add/remove since this is native and
> > manages all itself
> >
> > thoughts?
> >
> >
> >
> >
> >
> > 2018-03-12 14:01 GMT+01:00 Carlos Rovira <carlosrov...@apache.org>:
> >
> >> Hi,
> >>
> >> long thread and very useful read here. Since Jewel is very similar to
> MDL
> >> in adding/removing classes I want to comment here some things:
> >>
> >> 1.- I just changed jewel typenames to the constructor and things works
> ok,
> >> I could remove the createElement override
> >> 2.- I have into account the use of typenames as something inmutable (as
> >> part of definition of a component) and classNames as things that are
> put by
> >> developer, or change at runtime due to some user operation
> >>
> >> Then:
> >>
> >> 3.- Why not use classList [1] instead of create our own CSSClassList ?
> is
> >> well supported in the browsers we are targeting
> >>
> >> Something more "light" :)
> >>
> >> 4.- I know that order in html classes are not relevant, in the
> execution.
> >> And most of people here doesn't mind if typenames are before or after
> >> classNames. So hope this doesn't make any problem to anyone here:
> >> Can I change the code to put typeNames before classNames in
> >> computeFinalClassNames? I think this not affects anyone since is a small
> >> change and helps me to get organized classnames and identify things. I
> >> think is better to see in final html typeNames first then classNames so
> >> "inheritance" (to call it some way), could be easy detected by the eye
> >>
> >> Thanks
> >>
> >> Carlos
> >>
> >>
> >> [1] https://www.w3schools.com/Jsref/prop_element_classlist.asp
> >>
> >>
> >>
> >>
> >>
> >>
> >> --
> >> Carlos Rovira
> >> http://about.me/carlosrovira
> >>
> >>
> >
> >
> > --
> > Carlos Rovira
> > http://about.me/carlosrovira
>
>


-- 

Piotr Zarzycki

Patreon: *https://www.patreon.com/piotrzarzycki
<https://www.patreon.com/piotrzarzycki>*

Reply via email to