Hi Piotr,
I was reding from the begging and saw when the CSSClassList came.
I think Harbs solution is ok, the only problem I see is that in MDL, and
Jewel, we need to have functions per class that manages
add/remove of classes and override of compute function. That's what I don't
like of current solution.
As well I don't see in all the thread a mention to element.classList
for the two problems you mention, I expect very few lines in the
jewel-framework.css (css rules very close to frmaework code)
but anyway, I think debugging will not help you with something like this
when you have a line in code that sets display:block or
changes the elements classes.
For second, the code you're referring was not final in any way, I committed
since I want to fix and commit other things
I'm still trying and testing different approaches as I get to final
conclusions, so please don't take that as something final.
About searching vertical layout (if that would be final code), you only
need to search in jewel-framework.css that will only hold ClassReference
properties
in css rules and things like this (layouts) that are very close to the code
and will never go to a theme.
I think my approach is less monolithic since I'm separating concerns.
Piotr, I must say I'm very happy with today findings, but I need to
continue evolving it to avoid that you and others look to an unfinished
work
and take conclusions of things that will not be as they are currently
2018-03-12 17:58 GMT+01:00 Piotr Zarzycki <piotrzarzyck...@gmail.com>:
> 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>*
>
--
Carlos Rovira
http://about.me/carlosrovira
