Carlos, Are you saying here having your idea:
" 1) I think people have the APIs (className and classList) and can/will do what they want, although we say "use className only at init time". " If I do following things: <Component id="comp" className="myClass"/> And later in the code I do: comp.className = "myOtherClass"; It won't work? Piotr On Sat, Apr 14, 2018, 11:48 AM Carlos Rovira <carlosrov...@apache.org> wrote: > Alex > > 2018-04-14 8:41 GMT+02:00 Alex Harui <aha...@adobe.com.invalid>: > > > Carlos, > > > > It seems like either you have missed some of the discussion or maybe we > > weren't clear enough. > > > > I think most of what you say was considered but let's go for parts: > > > > > > Simply put: > > -The Basic components do not need to handle classList APIs. There is no > > expectation that classes will be frequently added and removed. > > > > That's ok. Basic no, Jewel yes. That's huge diference, so I think as you > UIBase should go to Core and be as is now (or make the changes you > estimate) > > > > -The goal of most component sets in Royale is to abstract away the > > underlying platform APIs. That's why I'm not in favor of having a > > classList API on UIBase. > > > > Right, classList is JS only so maybe an API in JewelUIBase should be > general and use the JS code with COMPILE::JS > then implement others. > > -MXML is better with space delimited string lists instead of arrays. > > > > I think in you version and my version is a string, but in mine then is > converted to feed classList > > > > -It doesn't make sense to split strings into an array in JS when the > > browser clearly can do it. > > -This perf test shows className is faster [1] > > -So does this one [2] > > > > > > We are starting from a list of strings. MDL is not. And that makes a > > difference, IMO. > > > > > ok in Jewel we could do in that way, the main difference with Basic is that > the main task of this kind of set is > heavily deal with class selectors so for Jewel (not for Basic) we should > focus on it what means to me be fundamental part of JewelUIBase to have an > API to deal with styles in all platforms and that all components extending > it can use. > > > > I forgot to mention earlier that I was not happy that addStyles and > > friends were JS-only. It would have been better if it did not take an > > element since that is a JS platform implementation. That way application > > developers could use addStyles and friends to manipulate the set of class > > selectors at runtime. > > > > ok, that's a fail on my implementation that should be fixed > > > > > > More comments in-line.. > > > > [1] > > https://measurethat.net/Benchmarks/Show/54/0/ > > classname-vs-setattribute-vs-c > > lasslist > > [2] https://jsperf.com/classname-vs-classlist-showdown/5 > > > > On 4/13/18, 7:18 PM, "carlos.rov...@gmail.com on behalf of Carlos > Rovira" > > <carlos.rov...@gmail.com on behalf of carlosrov...@apache.org> wrote: > > > > >Hi, > > > > > >I think the discussion now should center in numbers. > > > > > >I added "performance.now()" to typedefs (how could we live without this > > >until now? :)) > > > > Snip... > > > > > > > > >I think numbers are near identical right? > > > > > > > > >So given very close numbers should make us choose the more simplest code > > > > > > > > >right? > > > > > No. Small samples are often not useful. These kinds of arguments are > the > > ones that led to UIComponent being 13,000 lines in Flex. > > > > I think here we are talking about performance, not about to increase number > of lines of code > > > > > > > > > > > > > > >2018-04-14 2:58 GMT+02:00 Carlos Rovira <carlosrov...@apache.org>: > > > > > >> Hi Alex, > > >> > > >> just studied you changes and want to ask you a few things: > > >> > > >> 1) Why className and classLists methods must remain unsynced? I think > > >>this > > >> is not necessary and seems to me a bit unnatural since when I add > styles > > >> though classList in a element this makes the internal list changed, > and > > >>if > > >> I then do "trace(element.className)" it will report the updated > > >>list...so I > > >> think both are synced by default > > > > I proposed a way to have components that want to use classList pay for > it. > > If you want to further penalize those components in order to maintain > > synchronization of classList and className go ahead as long as it doesn't > > impact org.apache.flex.core.UIBase. Yes, the browser keeps className and > > classList in sync, but you are missing that the emphasized, primary and > > secondary selectors are not in the className list maintained by UIBase > and > > there is additional cost to doing so. > > > > > That's because when I refactored the code when you ask me to do so I tried > to make it > className dependent. I think the solution is in JewelUIBase wire all > through classList. > we'll have a tiny performance hit, that's right, but I think a uiset with > the purpose of Jewel > (theming - styling - goof looking) is the price that have to pay. a little > less performance than Basic > > > > > > > >> > > >> 2) Now Button has two new methods that must make various operations > with > > >> arrays (join, push, splice...), this means in almost all jewel > > >>components > > >> override at least computeFinalClassNames and insert new custom methods > > >>for > > >> add/toggle/remove and each one will make various operations: in the > > >>case of > > >> toggle will do a push or splice and then the normal classList toggle > > >> operation. > > >> > > It is probably possible to package up the code I added to Jewel Button > and > > make it re-usable without inserting a base class for all of Jewel. Or is > > absolutely every Jewel component going to need that code? If so, then > > maybe a common base class for all of Jewel makes sense. > > > > Yes, all components in Jewel is doing/will do heavy use of style API, so is > a must > in this set to have that api (while it's not in UIBase and Basic) > > > > > > Also, the code I added to Jewel Button is can be greatly simplified if > you > > assume folks will not directly set className after adding to parent. > > > > No, as MDL, this kind of components are based on heavily add and removal of > class selectors > users will pushing buttons, clicking checkboxes, and more, and part of that > actions will be to add/remove/toggle > class selectors in one or more components. > > That's what MDL does, that what Jewel does, and that's what any actual UI > set with focus on visuals will do > > > > > > > > >> 3) we are introducing a new array property per component to store what > > >> classList already do > > > > No, the array does not have to have as many elements as the classList. > > >> > > >> So for me we are introducing lots of complexity, with more code > splitter > > >> in every class, each one with more operations of array operations that > > >> finaly makes the same call I did. And generating complexity since > > >>className > > >> should be used by users only at init time and then use the rest of > > >> classList apis... > > > > That's PAYG. The classes that need it get the additional complexity. > And > > again, if we want to restrict setting classname after init in Jewel, > > that's totally fine with me and will simplify the code. > > > > The classes that need get the additional complexity, but at least we need > to > proxy all that in JewelUIBase since *all* Jewel components will use the > same code > > > > > >> > > >> The only difference for me is that you want to avoid the classList > > >>initial > > >> add method that in most of the cases will add from 1 or 2 classes and > > >>up to > > >> 3-4-5. I think normal components would have 3 on average... > > >> > > >> This related to lots of sites saying "use classList instead of > > >>className" > > >> and frameworks like MDL that are based only on classList , and all > > >>jsperfs > > >> (that although are not reflecting our concrete use case and use of > > >> classList, I think are completely valid on essence) makes me think > about > > >> how we have such different visions. > > >> > > >> So I must to say that as much as I want to see the advantages the > > >> approaches do not convince me... > > >> > > >> for me is more simple that all of that. > > >> > > >> 1) I think people have the APIs (className and classList) and can/will > > >>do > > >> what they want, although we say "use className only at init time". > > > > Again, we are abstracting the underlying implementation of how to set > > class selectors in most Royale component sets. That way, if we want to > > target other output we have fewer APIs to reproduce. We only need to > > reproduce the conceptual APIs. > > >> > > >> 2) I think we should have the most easy way to modify since browsers > are > > >> takin care of internal apis performance (or at least I'm confident > with > > >> that, like I was confident on flash player performance) > > >> > > >> 3) I don't like to introduce lots of code when maybe the basic usage > of > > >> classList can be even more efficient. I give various jsperf studies > out > > >> there but both Harbs and you didn't show me anyone that shows > className > > >>as > > >> a better option. > > >> > > >> 4) If we are introducing such complexity, wouldn't be better to remove > > >> completely classList and end that code with the new array property and > > >> array operations? I think it will be more performant and will remove > > >> complexity. > > >> > > >> 5) If I use that solution for jewel, I should introduce some > > >>intermediate > > >> class between UIBase and a Jewel Component where I can proxy all that > > >> methods that are now in button to avoid replicate in all jewel > > >>components. > > >> And by doing that, as I said before, I'll prefer to remove that > > >>complexity > > >> and go for simple classList manipulation since is the same that MDL > (to > > >> name a concrete and successful project that renders and performs > > >> magnificent) does [1] (I put button example just as an example since > > >> there's lots more) > > >> > > >> Sincerily, I'm not convinced with the results exposed here, and I was > > >> always thinking that I was not seeing something evident, but now I'm > > >>even > > >> more convinced that we should use classList without any rejection. > Even > > >>for > > >> the use of className in MXML, is ok since I proved you can transform > it > > >> without problem getting the string, splitting and introducing in the > > >> classList, and then opertating with is when needed without any > > >>performance > > >> significant problem. For me is more problematic all the code we want > to > > >> introduce to avoid possible performance problems that I and many > others > > >> don't see and that main web projects actually don't see and are used > all > > >> over the web. We should not be different in something that other has > > >> already adopted, and if people is using it, is because the browsers, > the > > >> standards and all the web wants all people using it, and for me is > what > > >> happen with classList. > > >> > > >> in resume. I still don't want to make this discussion longer. I think > we > > >> have different opinions on this particular subject and the greatness > of > > >> royale is that it doesn't mind since if you and Harbs are betting for > > >> className, we can remain Basic with the initial use (or the current > > >>one). > > >> For Jewel, I can bet for the same method MDL uses with classList and > as > > >>I > > >> must to refactor half of the Jewel components to extend from UIBase > > >> directly instead of Basic components counterparts, I can put a > > >>JewelUIBase > > >> piece between that uses classList in the way Jewel need. In fact > Jewel, > > >>and > > >> any of the modern UI sets (Semantic, MDL, Bootstrap, ...) depends > > >>heavily > > >> in class selector assignation, hence the use of classList as a general > > >> rule. So I think is natural to have this marked differentiation, while > > >>in > > >> Basic we should not expect people wants to deal with class selectors > in > > >>a > > >> heavy use. > > >> > > >> Maybe I can wrong, but sincerely, if so I can't see where, but I > firmly > > >> believe in that, and for me is a clear definition of Jewel needs. > > >> > > >> Thoughts? > > > > My proposal lets our MDL and Jewel components use classList heavily in a > > PAYG way. It can be simplified if we are going to restrict setting of > > className after adding to the DOM. If you want to see what the code > looks > > like with that assumption try making the changes or I will do it. > > > > Alex, I prefer you do this. My only requirements are: > > * Have a JewelUIBase instead of the same code in all components. > * As I said, className will not be untouched after addToParent. That's a > huge part in MDL and Jewel > both sets add/remove/toggle styles at runtime. So make the changes > modifying that rule in your mind. > * Is critical for me that components extending doesn't have to add new > methods that should be abstracted > in JewelUIBase and we can use basic API calls since we are talking about > heavy use (or principal use) in > this kind of components set. > > > > > > I think we have proof that className is faster for when we want to use it > > at init time. I would like to see if we can create APIs for manipulation > > the classList at runtime that isn't JS-only and asssumes there is an > > element so folks can use those APIs at runtime instead being tempted to > > change className. > > > > I think that's the clue. While Basic makes all the duty before add to > parent, MDL and Jewel are constantly > adding, toggling and removing class selectors, that why we must put a clear > line between how UIBase works > (mainly like it's now) and how Jewel works (using heavy use of classList > since is its nature, and 'll get rid completely of className use internaly) > > I hope we are reaching to something here. Could you change the > implementation taking into account the differences discussed here? > > thanks > > > > > > > My 2 cents, > > -Alex > > >> > > >> > > > > > > > > >-- > > >Carlos Rovira > > >https://na01.safelinks.protection.outlook.com/?url= > > http%3A%2F%2Fabout.me%2 > > >Fcarlosrovira&data=02%7C01%7Caharui%40adobe.com% > > 7C9fbf7c0d5e994a9acb6008d5 > > >a1ae2520%7Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C0% > > 7C636592691687691520&s > > >data=TR5G34hZMVutbPgcwAzTtNlFR0mQb8quhoBewhsOnSc%3D&reserved=0 > > > > > > > -- > Carlos Rovira > http://about.me/carlosrovira >
