Hi Alex, app devs will create logic that will use our encapsulated code (i.e: setting primary to true or false). Jewel as MDL has properties that uses to deal with true/false values to make it easy for the user to change a style. Under the hood, that is changing the "element.class" or "positioner.class". I think component's royale className property should be updated and not remain dirty since that would create lots of confusion for our users. If we do that folks should never operate again with "className", maybe it could not be a problem, although left className in a non valid state sound strange to me. If you think it can work, we can try it and see what happen... but seems to me that maybe we're making thing very complex when all could be more straight lined. Anyway, I'll wait for your changes. Thanks!
Carlos 2018-04-15 8:49 GMT+02:00 Alex Harui <aha...@adobe.com.invalid>: > IMO, Carlos still isn't understanding my point. My point is that the app > devs will not need to be setting className. They will be setting > properties like "emphasized", "secondary" and "primary" that can do > whatever it wants under the covers, and if we assume that folks will not > set (or are not supposed to set) className after init time, there are much > simpler ways to handle this situation. > > That said, I have another idea about how to allow "emphasized", > "secondary" and "primary" to use classList that might be simpler that what > I have committed so far. I will try to code it up tomorrow for others to > look at. > > Thanks, > -Alex > > On 4/14/18, 9:24 AM, "Piotr Zarzycki" <piotrzarzyck...@gmail.com> wrote: > > >Are you saying that it will work with your implementation and not with > >Alex's? > > > >Actually as a app developer above situation is very rare. > > > >Thanks, > >Piotr > > > >On Sat, Apr 14, 2018, 5:24 PM Carlos Rovira <carlosrov...@apache.org> > >wrote: > > > >> That's what Alex want. To make className only used at init time then we > >> should use classList methods. > >> I think that the premises are not right, since Alex thinks devs will not > >> make heavy use of switching class selectors at runtime (at init time and > >> later while using the app). MDL and Jewel are constantly setting and > >> removing class selectors from elements and positioners, so className > >>(set > >> only once at init time) is not right for use, but for people using > >>Basic. > >> Hence, Basic should remain with className, and MDL/Jewel should go > >> classList *always* (shadowing it so will have a royale API to work with > >>SWF > >> and JS and in JS compilation use classList > >> > >> 2018-04-14 14:24 GMT+02:00 Piotr Zarzycki <piotrzarzyck...@gmail.com>: > >> > >> > 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://na01.safelinks.protection.outlook.com/?url= > https%3A%2F%2Fmeasuret > >>hat.net%2FBenchmarks%2FShow%2F54%2F0%2F&data=02%7C01%7Caharui% > 40adobe.com > >>%7C18aef785013e484cfcf108d5a22445a7%7Cfa7b1b5a7b34438794aed2c178de > cee1%7C > >>0%7C0%7C636593199085882368&sdata=ya1hbY9%2FHxLCMbEh% > 2BVqXP0OdWrV03zEPRAtu > >>ybj223w%3D&reserved=0 > >> > > > classname-vs-setattribute-vs-c > >> > > > lasslist > >> > > > [2] > >>https://na01.safelinks.protection.outlook.com/?url= > https%3A%2F%2Fjsperf.c > >>om%2Fclassname-vs-classlist-showdown%2F5&data=02%7C01%7Caharui% > 40adobe.co > >>m%7C18aef785013e484cfcf108d5a22445a7%7Cfa7b1b5a7b34438794aed2c178de > cee1%7 > >>C0%7C0%7C636593199085882368&sdata=ygYm8sTMDTniu1NFzxds3fEaa0e% > 2FxnNaYxHeO > >>6G3F0g%3D&reserved=0 > >> > > > > >> > > > 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 > >> > > > >>https://na01.safelinks.protection.outlook.com/?url= > http%3A%2F%2Fabout.me% > >>2Fcarlosrovira&data=02%7C01%7Caharui%40adobe.com% > 7C18aef785013e484cfcf108 > >>d5a22445a7%7Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C0% > 7C63659319908588236 > >>8&sdata=P5v3BoebWGUeMXAsP5j7rd0y4rcjYPgYGKsAptIcLSQ%3D&reserved=0 > >> > > > >> > > >> > >> > >> > >> -- > >> Carlos Rovira > >> > >>https://na01.safelinks.protection.outlook.com/?url= > http%3A%2F%2Fabout.me% > >>2Fcarlosrovira&data=02%7C01%7Caharui%40adobe.com% > 7C18aef785013e484cfcf108 > >>d5a22445a7%7Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C0% > 7C63659319908588236 > >>8&sdata=P5v3BoebWGUeMXAsP5j7rd0y4rcjYPgYGKsAptIcLSQ%3D&reserved=0 > >> > > -- Carlos Rovira http://about.me/carlosrovira
