2018-03-15 18:06 GMT+01:00 Alex Harui <[email protected]>: > Did I miss where the actual code is? It is hard to see what code is added > to UIBase and what is in utility functions.
is in "feature/jewel-ui-set" branch, I think is better discuss here, and then decide when goes to develop > IMO, classList APIs should be > in utility functions, not in UIBase. > ok, I though the code you proposed was in UIBase, I checked now and notice you prefixed with a package I'll try refactor that > > Whether className or classList is faster isn't the key issue. The key > issue is how we can manage classnames, typenames, and things like "fab" > and "primary" and still let folks use classList.toggle. > > Thanks, > -Alex > > On 3/15/18, 9:32 AM, "[email protected] on behalf of Carlos Rovira" > <[email protected] on behalf of [email protected]> wrote: > > >2018-03-15 16:22 GMT+01:00 Harbs <[email protected]>: > > > >> A number of thoughts: > >> > >> 1. You did not address my question on whether using classList is really > >> more efficient. > >>https://na01.safelinks.protection.outlook.com/?url=https% > 3A%2F%2Fplus.goo > >>gle.com%2F%2BPaulIrish%2Fposts%2FAPArpwWqew3&data=02%7C01% > 7Caharui%40adob > >>e.com%7Cb0034aba92a946d7753e08d58a928948%7Cfa7b1b5a7b34438 > 794aed2c178dece > >>e1%7C0%7C0%7C636567284346705995&sdata=zwWYPXZgQ%2FQ3RZ7qPF > oYTCpUmTd50v4vy > >>ylykwwJKCA%3D&reserved=0 < > >> > >>https://na01.safelinks.protection.outlook.com/?url=https% > 3A%2F%2Fplus.goo > >>gle.com%2F%2BPaulIrish%2Fposts%2FAPArpwWqew3&data=02%7C01% > 7Caharui%40adob > >>e.com%7Cb0034aba92a946d7753e08d58a928948%7Cfa7b1b5a7b34438 > 794aed2c178dece > >>e1%7C0%7C0%7C636567284346705995&sdata=zwWYPXZgQ%2FQ3RZ7qPF > oYTCpUmTd50v4vy > >>ylykwwJKCA%3D&reserved=0> was published in > >> 2012. That was six years ago. In the meantime > >> > >>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%7Cb0034aba92a946d7753e08d58a928948%7Cfa7b1b5a7b34438794a > ed2c178decee1%7 > >>C0%7C0%7C636567284346705995&sdata=nymSi0q6QFKP98I%2F%2BhLq > h3RGWkmC4hnU1yE > >>clGNQSRc%3D&reserved=0 < > >> > >>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%7Cb0034aba92a946d7753e08d58a928948%7Cfa7b1b5a7b34438794a > ed2c178decee1%7 > >>C0%7C0%7C636567284346705995&sdata=nymSi0q6QFKP98I%2F%2BhLq > h3RGWkmC4hnU1yE > >>clGNQSRc%3D&reserved=0> seems to indicate > >> that just replacing the class name is much more efficient in modern > >> browsers. It’s possible that there’s something wrong with the test, but > >>it > >> looks accurate to me. > >> > > > >I doubt that after 6 years performs bad, I just expects the opposite. And > >if there's more test with one saying one performs better that other and > >then just the opposite, what this means to me is that at least both has > >same opportunities. > > > > > >> 2. Calling setClassName(computeFinalClassNames()); both when className > >> assigned and when addedToParent() is called will result in the > >>classNames > >> being set at least twice for every component. > >> > > > >That's where I want help. I put this layout as initial and hope you or > >Alex > >or someone suggest what to do to get the best performance with less > >callings > > > > > >> > >> 3. Adding “remove” and “removeAll” and “toggle" to every UIBase does not > >> seem very PAYG. > >> > > > >That's what we discussed in the other thread. For me is worth it since, > >Jewel is all about this (90% of the work is structuring html output and > >apply styles) > > > > > >> > >> 4. “removeAll” looks very inefficient. Simply zeroing out the className > >>of > >> the element should be much more efficient than looping over the > >>collection. > >> > > > >seems but not: > >https://na01.safelinks.protection.outlook.com/?url=https% > 3A%2F%2Fjsperf.co > >m%2Fclassname-vs-classlist-for-clear&data=02%7C01%7Caharui%40adobe.com > %7Cb > >0034aba92a946d7753e08d58a928948%7Cfa7b1b5a7b34438794aed2c17 > 8decee1%7C0%7C0 > >%7C636567284346705995&sdata=TYnkhROJg%2B%2FmJ9pmWpQEHNzXvN5 > f33MGTKKb%2B7wm > >wDU%3D&reserved=0 > >I talked about this in the other thread that this loops seems to perform > >better that doing className = "" > >I was the first to be surprised > >Maybe we should run our own test, by again seems classList is a perfect > >option to use. > > > > > >> 5. There’s no way to get the (full) classNames of a component using your > >> methods. > >> > >> > >trace(classNames) ? > >(or I'm not understanding you question) > > > > > >> 6. I don’t think there’s a need to use trim and even if there would, the > >> native JS String.prototype.trim() works fine. The StringUtil is only > >>really > >> useful for cross-platform code. > >> > > > >I found that not using it gives error when there's no a) typenNames or b) > >classNames and depending where you put the " " to join both > >the problem in that cases is that generates an array with one element and > >a > >space, that makes the runtime breaks with an error. For that reason I put > >the trim, but again, if you have a better solution propose a line of code > >to substitute. If the most we can get is change for the JS trim, I'll do > >that > > > > > >> > >> Harbs > >> > >> > On Mar 15, 2018, at 4:06 PM, Carlos Rovira <[email protected]> > >> wrote: > >> > > >> > Hi, > >> > > >> > I code the API we talked about in the previous email. Is in the UIBase > >> copy > >> > in Jewel lib > >> > > >> > Some things to notice as you review: > >> > > >> > * I put COMPILE::JS in each method, but maybe the methods should be > >>for > >> all > >> > platforms and use COMPILE::XX in the body > >> > > >> > * typeNames comes back as a public var without getter/setters > >> > > >> > * In addedToParent I call setClassName(computeFinalClassNames()) > >> > (this makes the components with properties reach sooner than this > >>call, > >> is > >> > there some way to make this reach the first?) > >> > > >> > * className setter as well calls > >>setClassName(computeFinalClassNames()) > >> > > >> > * computeFinalClassNames has a StringUtil.trim, maybe this could be a > >> > problem but don't know other way to ensure there's no spaces left, > >>since > >> if > >> > not this comes a problem for the latter split > >> > > >> > * setClassName calls new API "addStyles" > >> > > >> > * New API is composed of: > >> > addStyles -> is where all complicated logic is, this is the point > >>where > >> all > >> > converges, is this the best way to get this? some better alterantive? > >>let > >> > me know. Here I care for empty strings and for one style strings vs > >> > multiple separated by spaces > >> > removeStyles -> the counterpart to the previous method > >> > toggleStyles -> this is straight forward > >> > removeAllStyles -> This loop seems to be more performant than > >> className="" > >> > and I'm taking care of not removing typeNames in the process > >> > > >> > (I didn't create versions for contains and item classList methods > >>since > >> it > >> > seems not useful for us) > >> > > >> > Thoughts? > >> > > >> > > >> > -- > >> > Carlos Rovira > >> > > >>https://na01.safelinks.protection.outlook.com/?url=http% > 3A%2F%2Fabout.me% > >>2Fcarlosrovira&data=02%7C01%7Caharui%40adobe.com%7Cb0034ab > a92a946d7753e08 > >>d58a928948%7Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C0%7C63 > 656728434670599 > >>5&sdata=GfVk3KgQoaOSF3Q8yZ1GU3wJFRELRDMRCA1HvP1Qq64%3D&reserved=0 > >> > >> > > > > > >-- > >Carlos Rovira > >https://na01.safelinks.protection.outlook.com/?url=http%3A% > 2F%2Fabout.me%2 > >Fcarlosrovira&data=02%7C01%7Caharui%40adobe.com%7Cb0034aba9 > 2a946d7753e08d5 > >8a928948%7Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C0%7C63656 > 7284346705995&s > >data=GfVk3KgQoaOSF3Q8yZ1GU3wJFRELRDMRCA1HvP1Qq64%3D&reserved=0 > > -- > Carlos Rovira > http://about.me/carlosrovira > > >
