Hi Alex, if will be of help if you point us to different links where we can learn about this modifications, since I at least can just imagine what is all about, but will need to get deeper in the concepts to understand the changes and to apply this patterns.
In Jewel each "list component has its own type of renderer, so for example List uses ListItemRenderer and DataGrid has DataGridItemRenderer, since usually at that component level the user needs similar infrastructure like hoverable, selectable...and some (not much) more, don't know right now how all this will fit with the "has" new pattern....I'll try it. Just one important thing. There's actual users and clients using Jewel and other UI sets and are with very short times for their migrations, so just want to ask you to test as much as possible, since TDJ has many examples now. Other thing you can test is new TodoMVC to see how it behaves, since it uses a List with IRs. So we can ensure (as much as we can) the merge left things working (as much as we can) Thanks for working on this, will try all of this tomorrow Carlos El lun., 17 feb. 2020 a las 22:35, Alex Harui (<[email protected]>) escribió: > I've pushed the "has" branch that contains a refactoring of the item > renderers. Tour De Jewel and MDL Example seem to be working as is Basic > List Example and MX AdvancedDataGrid. > > "has" is really just calls to getBeadByType. If we start to see > performance issues, then we'll look into optimizing. The motivation to > switch to "has" came from several bugs about using MX Label/CheckBox as > item renderers. In Royale, the ItemRenderers were in control of the > visuals of their rollover and selected state. That is a more proper > encapsulation than the way it was in Flex where the lists drew the rollover > and selected states and it was hard to override the visuals for a custom > item renderer. But in the develop branch Royale code, it would require > that custom itemrenderers implement a lot of APIs related to rollover and > selected visuals. Instead we can now reuse/share code for visuals between > different renderers because a renderer now can "has" a rollover/selection > implementation instead of being one. > > There are more pieces involved, but there is more sharing of code. Along > the way I found that there were some not-so-PAYG patterns being used in MDL > and Jewel renderers that might deserve further modification. There are > "hoverable" and "selectable" APIs that appear to be used to permanently > turn off selection and hover visuals. In general, I think there is better > use of PAYG and composition when functionality is "built up" and not > "turned off", so with the "has" pattern the renderers can be added to a > list without any selection visuals at all (for a non-selectable list) or > re-composed with custom visuals that only support hoverable, for example. > I left it "hoverable/selectable" in the API surface for now, but something > to think about going forward. > > I’m going to run a few more tests before merging and pushing to develop. > > -Alex > > On 1/15/20, 10:44 PM, "Alex Harui" <[email protected]> wrote: > > You are welcome to try and see how many cache hits it gets. I think > in renderers, we ask once per renderer. I'm not sure there is a faster way > to do the first lookup of "is", but for "has" we could change the lookup > and save time. > > On 1/15/20, 10:38 PM, "Greg Dove" <[email protected]> wrote: > > For the 'something is ISomeInterface' > I had wondered in the past if these types of lookups could be > incrementally > cached on the 'is' target (after first lookup), that might make > sense for > interfaces, which are the deepest checks I think? > caching result (could optionally create the Map) > ISomeInterface['implMap'].set(something.constructor, isResult ) > > then earlier in the interface checks, it could do something like: > if (ISomeInterface['implMap'] && > ISomeInterface['implMap'].has(something.constructor) ) return > ISomeInterface['implMap'].get(something.constructor) > > I realize its extra code, but it should be quite a bit faster over > time I > think. > > On Thu, Jan 16, 2020 at 7:20 PM Alex Harui > <[email protected]> wrote: > > > Hi, > > > > Several different threads have brought up issues with sharing > code between > > component sets. Other threads have offered different and clever > ways to > > do various things like how MXML is applied to a component. > Meanwhile, over > > in MX emulation, I was starting to copy some code from Basic to > MXRoyale to > > get the various MX components to be valid item renderers. > MXRoyale is > > using Basic's item renderer architecture which is better > encapsulated: the > > renderer draws its hovered and selected state. In Flex, the > List draws > > over the renderer, which makes it hard to customize the way the > renderer > > will look when hovered and selected. > > > > It finally occurred to me that one of the reasons we end up > copying code > > is because we are still using too many "is" checks instead of > "has" > > checks. I'm not even sure we have any "has" checks in the Royale > > framework. I was afraid of the overhead of a "has" check, but > I'm starting > > to change my mind because: > > > > 1) The "is" check actually runs a fair amount of code, > especially for > > (comp is ISomeInterface) > > 2) The length of bead arrays don't seem too long. > > > > A "has" check calls getBeadByType(ISomeInterface), so it > actually will run > > the (bead is ISomeInterface) on potentially the entire strand > array/vector, > > although we could speed that up by annotating beads or keeping > track of > > what is on the strand. But the code sharing/reuse potential of > this > > pattern seems significant to me. > > > > For example, it could change how hard it is to make a component > usable as > > a top tag in MXML. Instead of the component having to implement > certain > > methods, the component could have a bead installed and the > > MXMLDataInterpreter could talk to that bead instead of the > component. > > > > In the case of the item renderers, instead of testing if the > renderer "is" > > ISelectableLIstItemRenderer, it could ask if the created widget > "has" an > > ISelectableLIstItemRenderer bead and the logic in that bead can > be reused > > in both Basic and MXRoyale without being copied. > > > > Some code, like Container overrides of addElement probably can't > be > > refactored into a "has". But I wonder how many other things > could. I'm > > not sure I would move everything that could be moved into a > shared bead. > > We'd have to think about the overhead on small components and > apps. But > > for MXML support and Item Renderer support, it seems to make > sense. > > > > Anyway, I will look into refactoring the item renderer code in > a few days > > unless feedback indicates otherwise. Bugs like #676 and #681 > inspired this > > post. > > > > Of course, I could be wrong... > > -Alex > > > > > > > > > -- Carlos Rovira http://about.me/carlosrovira
