Re: ItemRenderer is not PAYG

2018-04-15 Thread Alex Harui
I didn't do most of this code.  Although even if I did, it might be time
for some refactoring.  Thinking about it briefly:

Adding the ability to subclass in MXML is introduced at several points in
the SDK.  We should be choosing not to weigh down the lowest-level classes
with MXML capability (at least for now).

I think UIItemRendererBase doesn't need to have the MXML capability.  That
way, AS-based ItemRenderers will be a bit lighter and faster.
I think that the reason there are selectedColor, highlightColor, and
downColor properties is for backward compatibility with Flex item
renderers and maybe because there is no "down" or "selected" pseudo-states
in CSS, but I agree those can be moved off of UIItemRendererBase to some
other class. And the implementation of those color properties could just
set styles or class selectors.

I think assignable layout is not required in item renderers.  Simple ones
can just set x,y,width,height.   So maybe MXMLItemRenderer should add the
code that calls MXMLDataInterpreter and MXMLItemRendererWithLayout would
add the assignable layout support.

Thoughts?
-Alex

On 4/15/18, 9:36 AM, "Yishay Weiss"  wrote:

>I think we need to accept that there are some assumptions made in base
>classes that will not apply to every case. This is the tension between
>PAYG and reusability which has been discussed before. As Alex suggested
>you can always create a different implementation for
>ISelectableItemRenderer (or IItemRenderer).
>
>
>
>What I find confusing is that MXMLItemRenderer does not explicitly
>implement IMXMLDocument, and that most of the mxml related code is
>actually in UIItemRendererBase. Alex, maybe you can explain what the
>reasoning was for that?
>
>
>
>
>From: carlos.rov...@gmail.com  on behalf of
>Carlos Rovira 
>Sent: Sunday, April 15, 2018 2:29:20 PM
>To: dev@royale.apache.org
>Subject: Re: ItemRenderer is not PAYG
>
>Hi,
>
>the hierarchy chain is "UIItemRendererBase > DataItemRenderer >
>MXMLItemRenderer"
>
>ListItemRenderer extend from MXMLItemRenderer (if that's not the right
>class let me know), since users will want to create mainly MXML item
>renderers.
>
>So UIItemRendererBase is buried in the chain and I don't see a way to get
>rid of it unless I create the same chain.
>
>Being said that, this is not critical for me, I can live with those
>properties buried in the code, but I want to put this here to signal a
>point when I see PAYG is not begin followed.
>
>For me people that wants to use colors in code should "aggregate it" in a
>PAYG way, and people that wants css should do the same on its way.
>In other words, people using Jewel will have code in their apps that will
>be never used, and I think that's what we're trying to avoid
>
>
>
>
>
>
>
>2018-04-15 8:54 GMT+02:00 Alex Harui :
>
>> There is no obligation to use UIItemRendererBase for Jewel
>>ItemRenderers.
>> If you run into places where we assume UIItemRendererBase and not
>> IItemRenderer, that's either a bug or requires a different controller or
>> view.
>>
>> Let us know what you run into.
>>
>> -Alex
>>
>> On 4/14/18, 8:33 AM, "carlos.rov...@gmail.com on behalf of Carlos
>>Rovira"
>>  wrote:
>>
>> >Hi,
>> >
>> >this base class
>> >
>> >UIItemRendererBase
>> >
>> >has properties for all colors (hover, selected, and more) and a
>>"useColor"
>> >property, and updateRenderer() method is switching "useColor"
>> >
>> >as a low level class, I think this class should not have all this info,
>> >since most people will never use.
>> >
>> >In Basic I think is possible, but in Jewel colors, shapes and effects
>> >comer
>> >from CSS.
>> >
>> >In this case I think 95% of users will never go that way of setting
>>colors
>> >when the can do simply this:
>> >
>> >.jewel.item {
>> >cursor: pointer;
>> >padding: 8px;
>> >flex-shrink: 0;
>> >flex-grow: 1;
>> >}
>> >.jewel.item:hover {
>> >color: #FF;
>> >background: #24a3ef;
>> >}
>> >.jewel.item:active, .jewel.item.selected {
>> >color: #FF;
>> >background: #0f88d1;
>> >}
>> >
>> >without wiring a single line between logic and css.
>> >
>> >So I think useColors, and colors should be refactored to a bead or
>> >something that will not compromise the high level UI sets that will
>>never
>> >use this kind of properties.
>> >
>> >Although I'm creating a ItemRenderer from scratch, my problem here's
>>that
>> >there's so much hierarchy here and many other classes in the tree that
>> >depends.
>> >
>> >Creating a class extending "leaf" class nodes are easy, but when
>>problems
>> >arise in the middle of the hierarchy chain, we have a problem that is
>> >difficult to solve
>> >
>> >thanks
>> >
>> >
>> >
>> >--
>> >Carlos Rovira
>> >https://na01.safelinks.protection.outlook.com/?url=
>> http%3A%2F%2Fabout.me%2
>> >Fcarlosrovira=02%7C01%7Caharui%40adobe.com%
>> 

RE: ItemRenderer is not PAYG

2018-04-15 Thread Yishay Weiss
I think we need to accept that there are some assumptions made in base classes 
that will not apply to every case. This is the tension between PAYG and 
reusability which has been discussed before. As Alex suggested you can always 
create a different implementation for ISelectableItemRenderer (or 
IItemRenderer).



What I find confusing is that MXMLItemRenderer does not explicitly implement 
IMXMLDocument, and that most of the mxml related code is actually in 
UIItemRendererBase. Alex, maybe you can explain what the reasoning was for that?




From: carlos.rov...@gmail.com  on behalf of Carlos 
Rovira 
Sent: Sunday, April 15, 2018 2:29:20 PM
To: dev@royale.apache.org
Subject: Re: ItemRenderer is not PAYG

Hi,

the hierarchy chain is "UIItemRendererBase > DataItemRenderer >
MXMLItemRenderer"

ListItemRenderer extend from MXMLItemRenderer (if that's not the right
class let me know), since users will want to create mainly MXML item
renderers.

So UIItemRendererBase is buried in the chain and I don't see a way to get
rid of it unless I create the same chain.

Being said that, this is not critical for me, I can live with those
properties buried in the code, but I want to put this here to signal a
point when I see PAYG is not begin followed.

For me people that wants to use colors in code should "aggregate it" in a
PAYG way, and people that wants css should do the same on its way.
In other words, people using Jewel will have code in their apps that will
be never used, and I think that's what we're trying to avoid







2018-04-15 8:54 GMT+02:00 Alex Harui :

> There is no obligation to use UIItemRendererBase for Jewel ItemRenderers.
> If you run into places where we assume UIItemRendererBase and not
> IItemRenderer, that's either a bug or requires a different controller or
> view.
>
> Let us know what you run into.
>
> -Alex
>
> On 4/14/18, 8:33 AM, "carlos.rov...@gmail.com on behalf of Carlos Rovira"
>  wrote:
>
> >Hi,
> >
> >this base class
> >
> >UIItemRendererBase
> >
> >has properties for all colors (hover, selected, and more) and a "useColor"
> >property, and updateRenderer() method is switching "useColor"
> >
> >as a low level class, I think this class should not have all this info,
> >since most people will never use.
> >
> >In Basic I think is possible, but in Jewel colors, shapes and effects
> >comer
> >from CSS.
> >
> >In this case I think 95% of users will never go that way of setting colors
> >when the can do simply this:
> >
> >.jewel.item {
> >cursor: pointer;
> >padding: 8px;
> >flex-shrink: 0;
> >flex-grow: 1;
> >}
> >.jewel.item:hover {
> >color: #FF;
> >background: #24a3ef;
> >}
> >.jewel.item:active, .jewel.item.selected {
> >color: #FF;
> >background: #0f88d1;
> >}
> >
> >without wiring a single line between logic and css.
> >
> >So I think useColors, and colors should be refactored to a bead or
> >something that will not compromise the high level UI sets that will never
> >use this kind of properties.
> >
> >Although I'm creating a ItemRenderer from scratch, my problem here's that
> >there's so much hierarchy here and many other classes in the tree that
> >depends.
> >
> >Creating a class extending "leaf" class nodes are easy, but when problems
> >arise in the middle of the hierarchy chain, we have a problem that is
> >difficult to solve
> >
> >thanks
> >
> >
> >
> >--
> >Carlos Rovira
> >https://na01.safelinks.protection.outlook.com/?url=
> http%3A%2F%2Fabout.me%2
> >Fcarlosrovira=02%7C01%7Caharui%40adobe.com%
> 7C6594b31e04554af2d97808d5
> >a21d2617%7Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C0%
> 7C636593168509138978
> >data=q1GZeXydbyaKPui4RFXJG4%2FlUdCrTcioiycbxGE5FD4%3D=0
>
>


--
Carlos Rovira
http://about.me/carlosrovira


Re: [royale-asjs] 01/01: UIBase className changes to support new cssclassList class methods like addStyles

2018-04-15 Thread Carlos Rovira
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 :

> 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"  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 
> >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 :
> >>
> >> > 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:
> >> >
> >> > 
> >> >
> >> > 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  >
> >> > wrote:
> >> >
> >> > > Alex
> >> > >
> >> > > 2018-04-14 8:41 GMT+02:00 Alex Harui :
> >> > >
> >> > > > 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
> 

Re: ItemRenderer is not PAYG

2018-04-15 Thread Alex Harui
There is no obligation to use UIItemRendererBase for Jewel ItemRenderers.
If you run into places where we assume UIItemRendererBase and not
IItemRenderer, that's either a bug or requires a different controller or
view.

Let us know what you run into.

-Alex

On 4/14/18, 8:33 AM, "carlos.rov...@gmail.com on behalf of Carlos Rovira"
 wrote:

>Hi,
>
>this base class
>
>UIItemRendererBase
>
>has properties for all colors (hover, selected, and more) and a "useColor"
>property, and updateRenderer() method is switching "useColor"
>
>as a low level class, I think this class should not have all this info,
>since most people will never use.
>
>In Basic I think is possible, but in Jewel colors, shapes and effects
>comer
>from CSS.
>
>In this case I think 95% of users will never go that way of setting colors
>when the can do simply this:
>
>.jewel.item {
>cursor: pointer;
>padding: 8px;
>flex-shrink: 0;
>flex-grow: 1;
>}
>.jewel.item:hover {
>color: #FF;
>background: #24a3ef;
>}
>.jewel.item:active, .jewel.item.selected {
>color: #FF;
>background: #0f88d1;
>}
>
>without wiring a single line between logic and css.
>
>So I think useColors, and colors should be refactored to a bead or
>something that will not compromise the high level UI sets that will never
>use this kind of properties.
>
>Although I'm creating a ItemRenderer from scratch, my problem here's that
>there's so much hierarchy here and many other classes in the tree that
>depends.
>
>Creating a class extending "leaf" class nodes are easy, but when problems
>arise in the middle of the hierarchy chain, we have a problem that is
>difficult to solve
>
>thanks
>
>
>
>-- 
>Carlos Rovira
>https://na01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fabout.me%2
>Fcarlosrovira=02%7C01%7Caharui%40adobe.com%7C6594b31e04554af2d97808d5
>a21d2617%7Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C0%7C636593168509138978
>data=q1GZeXydbyaKPui4RFXJG4%2FlUdCrTcioiycbxGE5FD4%3D=0



Re: [royale-asjs] 01/01: UIBase className changes to support new cssclassList class methods like addStyles

2018-04-15 Thread Alex Harui
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"  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 
>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 :
>>
>> > 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:
>> >
>> > 
>> >
>> > 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 
>> > wrote:
>> >
>> > > Alex
>> > >
>> > > 2018-04-14 8:41 GMT+02:00 Alex Harui :
>> > >
>> > > > 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