Re: FlexJS element setter

[Piotr Zarzycki]

Hi Harbs,

I'm adding Royale dev list.

I just checked your changes with more complex app and apart of many
conflicts during merge with my custom SDK I don't see any problems. Take a
look into my comments to one of your commit. Once you do that from my sight
is green light to merge it to develop.

I'm sorry for a long delay!

Thanks, Piotr


2017-09-26 21:01 GMT+02:00 Piotr Zarzycki :

> Harbs,
>
> Give me couple of days and I will pick up that branch and try it out. I
> will also review those changes and give the feedback.
>
> Thanks!
>
> 2017-09-26 20:50 GMT+02:00 Harbs :
>
>> I think I’m done. Any reason to not merge into develop?
>>
>> > On Sep 26, 2017, at 7:01 PM, Piotr Zarzycki 
>> wrote:
>> >
>> > Harbs,
>> >
>> > Please push those changes into separate branch "feature/" no matter how
>> non
>> > serious it look. I hope your changes will simplify things.
>> >
>> > Thank you!
>> >
>> >
>> > 2017-09-26 17:54 GMT+02:00 Harbs :
>> >
>> >> I’m working on refactoring this.
>> >>
>> >> Is there a reason for the null check in UIBase.createElement()?
>> >>
>> >> Why would createElement be called if the element is already created?
>> None
>> >> of the subclasses have this null check.
>> >>
>> >> if (element == null)
>> >>element = document.createElement('div') as WrappedHTMLElement;
>> >>
>> >> Do you think it’s safe to remove the check?
>> >>
>> >>> On Sep 26, 2017, at 6:42 PM, Alex Harui 
>> >> wrote:
>> >>>
>> >>> I believe there are components where more than one HTMLElement is
>> created
>> >>> but only one is (and can be) assigned as "element" but all have
>> >>> flexjs_wrapper assigned to the wrapping IUIBase.
>> >>>
>> >>> If in fact no components need a separate positioner, it is fine to
>> remove
>> >>> it.  But if we keep it, even as a getter returning element, we have to
>> >>> make sure our code that positions things uses positioner and not
>> element
>> >>> in case someone does try to override positioner some day.
>> >>>
>> >>> As Peter mentioned, the original thinking was that element would be
>> the
>> >>> HTMLElement that defines the node in the DOM that dispatches
>> interaction
>> >>> events, but positioner might be some parent of the element like a Div
>> >> used
>> >>> to give the element appropriate visuals, chrome, accessory widgets,
>> etc,
>> >>> like NumericStepper, ComboBox, DateField, and maybe more sophisticated
>> >>> components like a RichTextEditor where the "element" is a Div that
>> gets
>> >>> focus and holds the text lines, but is a child of a positioner Div
>> that
>> >>> also contains child buttons for bold/italic/underline.  Another
>> example
>> >>> may be VideoPlayback.  The element might be some sort of video widget
>> but
>> >>> the positioner might be a div that also contains child buttons for
>> >>> stop/pause/rewind/forward.
>> >>>
>> >>> Of course, I could be wrong...
>> >>> -Alex
>> >>>
>> >>> On 9/26/17, 6:27 AM, "Peter Ent"  wrote:
>> >>>
>>  @Harbs: yes on get positioner returning element. This way someone
>> could
>>  override the getter and return something else if it suited their
>> needs.
>> 
>>  —peter
>> 
>>  On 9/26/17, 9:25 AM, "Harbs"  wrote:
>> 
>> > I looked at MDL and I don’t see any problem there.
>> >
>> > I’m talking about simplifying things across the board. I don’t see
>> how
>> >> it
>> > could effect anything.
>> >
>> > @Peter I think removing positioner might not be a bad idea, but
>> keeping
>> > it and using it as a pointer to element is basically just as cheap.
>> >
>> >> On Sep 26, 2017, at 4:12 PM, Piotr Zarzycki <
>> >> piotrzarzyck...@gmail.com>
>> >> wrote:
>> >>
>> >> Hi Harbs,
>> >>
>> >> If you will do such changes like moving to set flexjs_wrapper in
>> the
>> >> setter
>> >> of element - please make it on the separate branch. Let me test
>> with
>> >> my
>> >> app
>> >> whether MDL will not breaking up. I hope that we could avoid this
>> one,
>> >> even
>> >> if I think that it seems to be quite reasonable to do that.
>> >>
>> >> Can you for example do this only for your custom component not for
>> the
>> >> global IUIBase class ?
>> >>
>> >> Let see what Peter say.
>> >>
>> >> Thanks, Piotr
>> >>
>> >>
>> >> 2017-09-26 15:02 GMT+02:00 Harbs :
>> >>
>> >>> Yishay and I were working on drag/drop today and we were modifying
>> >> one
>> >>> of
>> >>> the classes you wrote for generating the drag image.
>> >>>
>> >>> The code can be simplified by using cloneNode() and stuffing the
>> >>> results
>> >>> into the element. The thing is, it does not work until you assign
>> the
>> >>> flexjs_wrapper to the element. IMO, calling the element setter
>> 

Re: FlexJS element setter

[Piotr Zarzycki]

Harbs,

Give me couple of days and I will pick up that branch and try it out. I
will also review those changes and give the feedback.

Thanks!

2017-09-26 20:50 GMT+02:00 Harbs :

> I think I’m done. Any reason to not merge into develop?
>
> > On Sep 26, 2017, at 7:01 PM, Piotr Zarzycki 
> wrote:
> >
> > Harbs,
> >
> > Please push those changes into separate branch "feature/" no matter how
> non
> > serious it look. I hope your changes will simplify things.
> >
> > Thank you!
> >
> >
> > 2017-09-26 17:54 GMT+02:00 Harbs :
> >
> >> I’m working on refactoring this.
> >>
> >> Is there a reason for the null check in UIBase.createElement()?
> >>
> >> Why would createElement be called if the element is already created?
> None
> >> of the subclasses have this null check.
> >>
> >> if (element == null)
> >>element = document.createElement('div') as WrappedHTMLElement;
> >>
> >> Do you think it’s safe to remove the check?
> >>
> >>> On Sep 26, 2017, at 6:42 PM, Alex Harui 
> >> wrote:
> >>>
> >>> I believe there are components where more than one HTMLElement is
> created
> >>> but only one is (and can be) assigned as "element" but all have
> >>> flexjs_wrapper assigned to the wrapping IUIBase.
> >>>
> >>> If in fact no components need a separate positioner, it is fine to
> remove
> >>> it.  But if we keep it, even as a getter returning element, we have to
> >>> make sure our code that positions things uses positioner and not
> element
> >>> in case someone does try to override positioner some day.
> >>>
> >>> As Peter mentioned, the original thinking was that element would be the
> >>> HTMLElement that defines the node in the DOM that dispatches
> interaction
> >>> events, but positioner might be some parent of the element like a Div
> >> used
> >>> to give the element appropriate visuals, chrome, accessory widgets,
> etc,
> >>> like NumericStepper, ComboBox, DateField, and maybe more sophisticated
> >>> components like a RichTextEditor where the "element" is a Div that gets
> >>> focus and holds the text lines, but is a child of a positioner Div that
> >>> also contains child buttons for bold/italic/underline.  Another example
> >>> may be VideoPlayback.  The element might be some sort of video widget
> but
> >>> the positioner might be a div that also contains child buttons for
> >>> stop/pause/rewind/forward.
> >>>
> >>> Of course, I could be wrong...
> >>> -Alex
> >>>
> >>> On 9/26/17, 6:27 AM, "Peter Ent"  wrote:
> >>>
>  @Harbs: yes on get positioner returning element. This way someone
> could
>  override the getter and return something else if it suited their
> needs.
> 
>  —peter
> 
>  On 9/26/17, 9:25 AM, "Harbs"  wrote:
> 
> > I looked at MDL and I don’t see any problem there.
> >
> > I’m talking about simplifying things across the board. I don’t see
> how
> >> it
> > could effect anything.
> >
> > @Peter I think removing positioner might not be a bad idea, but
> keeping
> > it and using it as a pointer to element is basically just as cheap.
> >
> >> On Sep 26, 2017, at 4:12 PM, Piotr Zarzycki <
> >> piotrzarzyck...@gmail.com>
> >> wrote:
> >>
> >> Hi Harbs,
> >>
> >> If you will do such changes like moving to set flexjs_wrapper in the
> >> setter
> >> of element - please make it on the separate branch. Let me test with
> >> my
> >> app
> >> whether MDL will not breaking up. I hope that we could avoid this
> one,
> >> even
> >> if I think that it seems to be quite reasonable to do that.
> >>
> >> Can you for example do this only for your custom component not for
> the
> >> global IUIBase class ?
> >>
> >> Let see what Peter say.
> >>
> >> Thanks, Piotr
> >>
> >>
> >> 2017-09-26 15:02 GMT+02:00 Harbs :
> >>
> >>> Yishay and I were working on drag/drop today and we were modifying
> >> one
> >>> of
> >>> the classes you wrote for generating the drag image.
> >>>
> >>> The code can be simplified by using cloneNode() and stuffing the
> >>> results
> >>> into the element. The thing is, it does not work until you assign
> the
> >>> flexjs_wrapper to the element. IMO, calling the element setter
> should
> >>> do
> >>> that automatically.
> >>>
> >>> On a similar note, Every IUIBase object has a positioner set. I
> don’t
> >>> know
> >>> of a single class which has a different positioner than the
> element.
> >>> It
> >>> seems to me that positioner should be a getter (which normally
> >> returns
> >>> the
> >>> element) that’s overridden for classes which need a different one.
> >>> That
> >>> will save memory for every IUIBase created.
> >>>
> >>> Harbs
> >>>
>  On Sep 26, 2017, at 3:23 PM, 

Re: FlexJS element setter

[Harbs]

I think I’m done. Any reason to not merge into develop?

> On Sep 26, 2017, at 7:01 PM, Piotr Zarzycki  wrote:
> 
> Harbs,
> 
> Please push those changes into separate branch "feature/" no matter how non
> serious it look. I hope your changes will simplify things.
> 
> Thank you!
> 
> 
> 2017-09-26 17:54 GMT+02:00 Harbs :
> 
>> I’m working on refactoring this.
>> 
>> Is there a reason for the null check in UIBase.createElement()?
>> 
>> Why would createElement be called if the element is already created? None
>> of the subclasses have this null check.
>> 
>> if (element == null)
>>element = document.createElement('div') as WrappedHTMLElement;
>> 
>> Do you think it’s safe to remove the check?
>> 
>>> On Sep 26, 2017, at 6:42 PM, Alex Harui 
>> wrote:
>>> 
>>> I believe there are components where more than one HTMLElement is created
>>> but only one is (and can be) assigned as "element" but all have
>>> flexjs_wrapper assigned to the wrapping IUIBase.
>>> 
>>> If in fact no components need a separate positioner, it is fine to remove
>>> it.  But if we keep it, even as a getter returning element, we have to
>>> make sure our code that positions things uses positioner and not element
>>> in case someone does try to override positioner some day.
>>> 
>>> As Peter mentioned, the original thinking was that element would be the
>>> HTMLElement that defines the node in the DOM that dispatches interaction
>>> events, but positioner might be some parent of the element like a Div
>> used
>>> to give the element appropriate visuals, chrome, accessory widgets, etc,
>>> like NumericStepper, ComboBox, DateField, and maybe more sophisticated
>>> components like a RichTextEditor where the "element" is a Div that gets
>>> focus and holds the text lines, but is a child of a positioner Div that
>>> also contains child buttons for bold/italic/underline.  Another example
>>> may be VideoPlayback.  The element might be some sort of video widget but
>>> the positioner might be a div that also contains child buttons for
>>> stop/pause/rewind/forward.
>>> 
>>> Of course, I could be wrong...
>>> -Alex
>>> 
>>> On 9/26/17, 6:27 AM, "Peter Ent"  wrote:
>>> 
 @Harbs: yes on get positioner returning element. This way someone could
 override the getter and return something else if it suited their needs.
 
 —peter
 
 On 9/26/17, 9:25 AM, "Harbs"  wrote:
 
> I looked at MDL and I don’t see any problem there.
> 
> I’m talking about simplifying things across the board. I don’t see how
>> it
> could effect anything.
> 
> @Peter I think removing positioner might not be a bad idea, but keeping
> it and using it as a pointer to element is basically just as cheap.
> 
>> On Sep 26, 2017, at 4:12 PM, Piotr Zarzycki <
>> piotrzarzyck...@gmail.com>
>> wrote:
>> 
>> Hi Harbs,
>> 
>> If you will do such changes like moving to set flexjs_wrapper in the
>> setter
>> of element - please make it on the separate branch. Let me test with
>> my
>> app
>> whether MDL will not breaking up. I hope that we could avoid this one,
>> even
>> if I think that it seems to be quite reasonable to do that.
>> 
>> Can you for example do this only for your custom component not for the
>> global IUIBase class ?
>> 
>> Let see what Peter say.
>> 
>> Thanks, Piotr
>> 
>> 
>> 2017-09-26 15:02 GMT+02:00 Harbs :
>> 
>>> Yishay and I were working on drag/drop today and we were modifying
>> one
>>> of
>>> the classes you wrote for generating the drag image.
>>> 
>>> The code can be simplified by using cloneNode() and stuffing the
>>> results
>>> into the element. The thing is, it does not work until you assign the
>>> flexjs_wrapper to the element. IMO, calling the element setter should
>>> do
>>> that automatically.
>>> 
>>> On a similar note, Every IUIBase object has a positioner set. I don’t
>>> know
>>> of a single class which has a different positioner than the element.
>>> It
>>> seems to me that positioner should be a getter (which normally
>> returns
>>> the
>>> element) that’s overridden for classes which need a different one.
>>> That
>>> will save memory for every IUIBase created.
>>> 
>>> Harbs
>>> 
 On Sep 26, 2017, at 3:23 PM, Peter Ent 
 wrote:
 
 The setter for element is in HTMLElementWrapper, the super class for
 UIBase. The setter for flexes_wrapper is in UIBase. So if the
>> element
 setter were to also set the flexjs_wrapper, it would have to be an
 override in UIBase to do it. At least that¹s how I understand it.
 
 Could you elaborate a little more on the issue that is raising 

Re: FlexJS element setter

[Harbs]

No. I just used it because it’s the simplest UI component. I don’t remember off 
hand what I needed it for.

> On Sep 26, 2017, at 8:59 PM, Alex Harui  wrote:
> 
> You aren't the only one, but why did you do it?  Is there some subclass of
> UIBase missing from the component set?  ScratchPad?  Spacer?
> 
> On 9/26/17, 10:53 AM, "Harbs"  wrote:
> 
>> I’ve used it.
>> 
>>> On Sep 26, 2017, at 8:41 PM, Alex Harui 
>>> wrote:
>>> 
>>> IMO, the first question is, do we really need to support "new UIBase()"?
>> 
> 

Re: FlexJS element setter

[Alex Harui]

You aren't the only one, but why did you do it?  Is there some subclass of
UIBase missing from the component set?  ScratchPad?  Spacer?

On 9/26/17, 10:53 AM, "Harbs"  wrote:

>I’ve used it.
>
>> On Sep 26, 2017, at 8:41 PM, Alex Harui 
>>wrote:
>> 
>> IMO, the first question is, do we really need to support "new UIBase()"?
>

Re: FlexJS element setter

[Harbs]

I’ve used it.

> On Sep 26, 2017, at 8:41 PM, Alex Harui  wrote:
> 
> IMO, the first question is, do we really need to support "new UIBase()"?

Re: FlexJS element setter

[Harbs]

Funny you mention that. I added addElementToWrapper() as a top level function 
which attaches elements to UIBases. I have just finished implementing this in 
all Basic and MDL components.

That pretty much removes the need to call super.

I’m committing my changes soon (to a branch).

> On Sep 26, 2017, at 8:41 PM, Alex Harui  wrote:
> 
> IMO, the first question is, do we really need to support "new UIBase()"?
> I remember being surprised in regular Flex when folks did "new
> UIComponent()".  There aren't abstract classes in ActionScript, but I
> would have made UIComponent and UIBase abstract if I could.
> 
> But if the answer is that we want to allow "new UIBase()" and expect it to
> generate a child HTMLElement, then how do we want to instruct folks to
> write their createElement overrides?  A common override pattern is this:
> 
> public class MyClass extends UIBase {
> 
>  override protected function createElement(){
>element = document.createElement("input");
>super.createElement();
>  }
> }
> 
> Or this:
> 
> public class MyClass extends UIBase {
> 
>  override protected function createElement(){
>super.createElement();
>element = document.createElement("input");
>  }
> }
> 
> 
> Maybe the API is poorly designed. Maybe UIBase.createElement() should take
> a String and UIBase would call it with 'div' and subclasses can change the
> 'div' to something else before calling super.createElement.
> 
> Of course, I could be wrong...
> -Alex
> 
> 
> 
> On 9/26/17, 10:21 AM, "Harbs"  wrote:
> 
>> Huh? Why would the subclass call super.createElement() and assume the
>> element will not be created?
>> 
>> FWIW, super.createElement() is barely called, and when it is (from all
>> the cases I’ve found so far in the whole Basic and MDL), it’s expecting
>> the default div element.
>> 
>>> On Sep 26, 2017, at 8:15 PM, Alex Harui 
>>> wrote:
>>> 
>>> IIRC, UIBase has that code so if you do "new UIBase()" you will get an
>>> element, but if subclasses call super.createElement() by habit, we won't
>>> overwrite anybody's element.  It isn't truly PAYG, it depends on how
>>> folks
>>> feel about "requiring" that you know not to call super.createElement()
>>> on
>>> UIBase.
>>> 
>>> I don't have an opinion on what is "right".
>>> 
>>> -Alex
>>> 
>>> On 9/26/17, 8:54 AM, "Harbs"  wrote:
>>> 
 I’m working on refactoring this.
 
 Is there a reason for the null check in UIBase.createElement()?
 
 Why would createElement be called if the element is already created?
 None
 of the subclasses have this null check.
 
 if (element == null)
  element = document.createElement('div') as WrappedHTMLElement;
 
 Do you think it’s safe to remove the check?
 
> On Sep 26, 2017, at 6:42 PM, Alex Harui 
> wrote:
> 
> I believe there are components where more than one HTMLElement is
> created
> but only one is (and can be) assigned as "element" but all have
> flexjs_wrapper assigned to the wrapping IUIBase.
> 
> If in fact no components need a separate positioner, it is fine to
> remove
> it.  But if we keep it, even as a getter returning element, we have to
> make sure our code that positions things uses positioner and not
> element
> in case someone does try to override positioner some day.
> 
> As Peter mentioned, the original thinking was that element would be
> the
> HTMLElement that defines the node in the DOM that dispatches
> interaction
> events, but positioner might be some parent of the element like a Div
> used
> to give the element appropriate visuals, chrome, accessory widgets,
> etc,
> like NumericStepper, ComboBox, DateField, and maybe more sophisticated
> components like a RichTextEditor where the "element" is a Div that
> gets
> focus and holds the text lines, but is a child of a positioner Div
> that
> also contains child buttons for bold/italic/underline.  Another
> example
> may be VideoPlayback.  The element might be some sort of video widget
> but
> the positioner might be a div that also contains child buttons for
> stop/pause/rewind/forward.
> 
> Of course, I could be wrong...
> -Alex
> 
> On 9/26/17, 6:27 AM, "Peter Ent"  wrote:
> 
>> @Harbs: yes on get positioner returning element. This way someone
>> could
>> override the getter and return something else if it suited their
>> needs.
>> 
>> —peter
>> 
>> On 9/26/17, 9:25 AM, "Harbs"  wrote:
>> 
>>> I looked at MDL and I don’t see any problem there.
>>> 
>>> I’m talking about simplifying things across the board. I don’t see
>>> how it
>>> could effect anything.
>>> 
>>> @Peter I think removing positioner might not 

Re: FlexJS element setter

[Alex Harui]

IIRC, UIBase has that code so if you do "new UIBase()" you will get an
element, but if subclasses call super.createElement() by habit, we won't
overwrite anybody's element.  It isn't truly PAYG, it depends on how folks
feel about "requiring" that you know not to call super.createElement() on
UIBase.

I don't have an opinion on what is "right".

-Alex

On 9/26/17, 8:54 AM, "Harbs"  wrote:

>I’m working on refactoring this.
>
>Is there a reason for the null check in UIBase.createElement()?
>
>Why would createElement be called if the element is already created? None
>of the subclasses have this null check.
>
>if (element == null)
>element = document.createElement('div') as WrappedHTMLElement;
>
>Do you think it’s safe to remove the check?
>
>> On Sep 26, 2017, at 6:42 PM, Alex Harui 
>>wrote:
>> 
>> I believe there are components where more than one HTMLElement is
>>created
>> but only one is (and can be) assigned as "element" but all have
>> flexjs_wrapper assigned to the wrapping IUIBase.
>> 
>> If in fact no components need a separate positioner, it is fine to
>>remove
>> it.  But if we keep it, even as a getter returning element, we have to
>> make sure our code that positions things uses positioner and not element
>> in case someone does try to override positioner some day.
>> 
>> As Peter mentioned, the original thinking was that element would be the
>> HTMLElement that defines the node in the DOM that dispatches interaction
>> events, but positioner might be some parent of the element like a Div
>>used
>> to give the element appropriate visuals, chrome, accessory widgets, etc,
>> like NumericStepper, ComboBox, DateField, and maybe more sophisticated
>> components like a RichTextEditor where the "element" is a Div that gets
>> focus and holds the text lines, but is a child of a positioner Div that
>> also contains child buttons for bold/italic/underline.  Another example
>> may be VideoPlayback.  The element might be some sort of video widget
>>but
>> the positioner might be a div that also contains child buttons for
>> stop/pause/rewind/forward.
>> 
>> Of course, I could be wrong...
>> -Alex
>> 
>> On 9/26/17, 6:27 AM, "Peter Ent"  wrote:
>> 
>>> @Harbs: yes on get positioner returning element. This way someone could
>>> override the getter and return something else if it suited their needs.
>>> 
>>> —peter
>>> 
>>> On 9/26/17, 9:25 AM, "Harbs"  wrote:
>>> 
 I looked at MDL and I don’t see any problem there.
 
 I’m talking about simplifying things across the board. I don’t see
how it
 could effect anything.
 
 @Peter I think removing positioner might not be a bad idea, but
keeping
 it and using it as a pointer to element is basically just as cheap.
 
> On Sep 26, 2017, at 4:12 PM, Piotr Zarzycki
>
> wrote:
> 
> Hi Harbs,
> 
> If you will do such changes like moving to set flexjs_wrapper in the
> setter
> of element - please make it on the separate branch. Let me test with
>my
> app
> whether MDL will not breaking up. I hope that we could avoid this
>one,
> even
> if I think that it seems to be quite reasonable to do that.
> 
> Can you for example do this only for your custom component not for
>the
> global IUIBase class ?
> 
> Let see what Peter say.
> 
> Thanks, Piotr
> 
> 
> 2017-09-26 15:02 GMT+02:00 Harbs :
> 
>> Yishay and I were working on drag/drop today and we were modifying
>>one
>> of
>> the classes you wrote for generating the drag image.
>> 
>> The code can be simplified by using cloneNode() and stuffing the
>> results
>> into the element. The thing is, it does not work until you assign
>>the
>> flexjs_wrapper to the element. IMO, calling the element setter
>>should
>> do
>> that automatically.
>> 
>> On a similar note, Every IUIBase object has a positioner set. I
>>don’t
>> know
>> of a single class which has a different positioner than the element.
>> It
>> seems to me that positioner should be a getter (which normally
>>returns
>> the
>> element) that’s overridden for classes which need a different one.
>> That
>> will save memory for every IUIBase created.
>> 
>> Harbs
>> 
>>> On Sep 26, 2017, at 3:23 PM, Peter Ent 
>>> wrote:
>>> 
>>> The setter for element is in HTMLElementWrapper, the super class
>>>for
>>> UIBase. The setter for flexes_wrapper is in UIBase. So if the
>>>element
>>> setter were to also set the flexjs_wrapper, it would have to be an
>>> override in UIBase to do it. At least that¹s how I understand it.
>>> 
>>> Could you elaborate a little more on the issue that is raising this
>>> concern?

Re: FlexJS element setter

[Harbs]

FYI: TableRowItemRenderer in MDL has a separate positioner.

> On Sep 26, 2017, at 7:01 PM, Piotr Zarzycki  wrote:
> 
> Harbs,
> 
> Please push those changes into separate branch "feature/" no matter how non
> serious it look. I hope your changes will simplify things.
> 
> Thank you!
> 
> 
> 2017-09-26 17:54 GMT+02:00 Harbs :
> 
>> I’m working on refactoring this.
>> 
>> Is there a reason for the null check in UIBase.createElement()?
>> 
>> Why would createElement be called if the element is already created? None
>> of the subclasses have this null check.
>> 
>> if (element == null)
>>element = document.createElement('div') as WrappedHTMLElement;
>> 
>> Do you think it’s safe to remove the check?
>> 
>>> On Sep 26, 2017, at 6:42 PM, Alex Harui 
>> wrote:
>>> 
>>> I believe there are components where more than one HTMLElement is created
>>> but only one is (and can be) assigned as "element" but all have
>>> flexjs_wrapper assigned to the wrapping IUIBase.
>>> 
>>> If in fact no components need a separate positioner, it is fine to remove
>>> it.  But if we keep it, even as a getter returning element, we have to
>>> make sure our code that positions things uses positioner and not element
>>> in case someone does try to override positioner some day.
>>> 
>>> As Peter mentioned, the original thinking was that element would be the
>>> HTMLElement that defines the node in the DOM that dispatches interaction
>>> events, but positioner might be some parent of the element like a Div
>> used
>>> to give the element appropriate visuals, chrome, accessory widgets, etc,
>>> like NumericStepper, ComboBox, DateField, and maybe more sophisticated
>>> components like a RichTextEditor where the "element" is a Div that gets
>>> focus and holds the text lines, but is a child of a positioner Div that
>>> also contains child buttons for bold/italic/underline.  Another example
>>> may be VideoPlayback.  The element might be some sort of video widget but
>>> the positioner might be a div that also contains child buttons for
>>> stop/pause/rewind/forward.
>>> 
>>> Of course, I could be wrong...
>>> -Alex
>>> 
>>> On 9/26/17, 6:27 AM, "Peter Ent"  wrote:
>>> 
 @Harbs: yes on get positioner returning element. This way someone could
 override the getter and return something else if it suited their needs.
 
 —peter
 
 On 9/26/17, 9:25 AM, "Harbs"  wrote:
 
> I looked at MDL and I don’t see any problem there.
> 
> I’m talking about simplifying things across the board. I don’t see how
>> it
> could effect anything.
> 
> @Peter I think removing positioner might not be a bad idea, but keeping
> it and using it as a pointer to element is basically just as cheap.
> 
>> On Sep 26, 2017, at 4:12 PM, Piotr Zarzycki <
>> piotrzarzyck...@gmail.com>
>> wrote:
>> 
>> Hi Harbs,
>> 
>> If you will do such changes like moving to set flexjs_wrapper in the
>> setter
>> of element - please make it on the separate branch. Let me test with
>> my
>> app
>> whether MDL will not breaking up. I hope that we could avoid this one,
>> even
>> if I think that it seems to be quite reasonable to do that.
>> 
>> Can you for example do this only for your custom component not for the
>> global IUIBase class ?
>> 
>> Let see what Peter say.
>> 
>> Thanks, Piotr
>> 
>> 
>> 2017-09-26 15:02 GMT+02:00 Harbs :
>> 
>>> Yishay and I were working on drag/drop today and we were modifying
>> one
>>> of
>>> the classes you wrote for generating the drag image.
>>> 
>>> The code can be simplified by using cloneNode() and stuffing the
>>> results
>>> into the element. The thing is, it does not work until you assign the
>>> flexjs_wrapper to the element. IMO, calling the element setter should
>>> do
>>> that automatically.
>>> 
>>> On a similar note, Every IUIBase object has a positioner set. I don’t
>>> know
>>> of a single class which has a different positioner than the element.
>>> It
>>> seems to me that positioner should be a getter (which normally
>> returns
>>> the
>>> element) that’s overridden for classes which need a different one.
>>> That
>>> will save memory for every IUIBase created.
>>> 
>>> Harbs
>>> 
 On Sep 26, 2017, at 3:23 PM, Peter Ent 
 wrote:
 
 The setter for element is in HTMLElementWrapper, the super class for
 UIBase. The setter for flexes_wrapper is in UIBase. So if the
>> element
 setter were to also set the flexjs_wrapper, it would have to be an
 override in UIBase to do it. At least that¹s how I understand it.
 
 Could you elaborate a little more on the issue that is 

Re: FlexJS element setter

[Piotr Zarzycki]

Harbs,

Please push those changes into separate branch "feature/" no matter how non
serious it look. I hope your changes will simplify things.

Thank you!


2017-09-26 17:54 GMT+02:00 Harbs :

> I’m working on refactoring this.
>
> Is there a reason for the null check in UIBase.createElement()?
>
> Why would createElement be called if the element is already created? None
> of the subclasses have this null check.
>
> if (element == null)
> element = document.createElement('div') as WrappedHTMLElement;
>
> Do you think it’s safe to remove the check?
>
> > On Sep 26, 2017, at 6:42 PM, Alex Harui 
> wrote:
> >
> > I believe there are components where more than one HTMLElement is created
> > but only one is (and can be) assigned as "element" but all have
> > flexjs_wrapper assigned to the wrapping IUIBase.
> >
> > If in fact no components need a separate positioner, it is fine to remove
> > it.  But if we keep it, even as a getter returning element, we have to
> > make sure our code that positions things uses positioner and not element
> > in case someone does try to override positioner some day.
> >
> > As Peter mentioned, the original thinking was that element would be the
> > HTMLElement that defines the node in the DOM that dispatches interaction
> > events, but positioner might be some parent of the element like a Div
> used
> > to give the element appropriate visuals, chrome, accessory widgets, etc,
> > like NumericStepper, ComboBox, DateField, and maybe more sophisticated
> > components like a RichTextEditor where the "element" is a Div that gets
> > focus and holds the text lines, but is a child of a positioner Div that
> > also contains child buttons for bold/italic/underline.  Another example
> > may be VideoPlayback.  The element might be some sort of video widget but
> > the positioner might be a div that also contains child buttons for
> > stop/pause/rewind/forward.
> >
> > Of course, I could be wrong...
> > -Alex
> >
> > On 9/26/17, 6:27 AM, "Peter Ent"  wrote:
> >
> >> @Harbs: yes on get positioner returning element. This way someone could
> >> override the getter and return something else if it suited their needs.
> >>
> >> —peter
> >>
> >> On 9/26/17, 9:25 AM, "Harbs"  wrote:
> >>
> >>> I looked at MDL and I don’t see any problem there.
> >>>
> >>> I’m talking about simplifying things across the board. I don’t see how
> it
> >>> could effect anything.
> >>>
> >>> @Peter I think removing positioner might not be a bad idea, but keeping
> >>> it and using it as a pointer to element is basically just as cheap.
> >>>
>  On Sep 26, 2017, at 4:12 PM, Piotr Zarzycki <
> piotrzarzyck...@gmail.com>
>  wrote:
> 
>  Hi Harbs,
> 
>  If you will do such changes like moving to set flexjs_wrapper in the
>  setter
>  of element - please make it on the separate branch. Let me test with
> my
>  app
>  whether MDL will not breaking up. I hope that we could avoid this one,
>  even
>  if I think that it seems to be quite reasonable to do that.
> 
>  Can you for example do this only for your custom component not for the
>  global IUIBase class ?
> 
>  Let see what Peter say.
> 
>  Thanks, Piotr
> 
> 
>  2017-09-26 15:02 GMT+02:00 Harbs :
> 
> > Yishay and I were working on drag/drop today and we were modifying
> one
> > of
> > the classes you wrote for generating the drag image.
> >
> > The code can be simplified by using cloneNode() and stuffing the
> > results
> > into the element. The thing is, it does not work until you assign the
> > flexjs_wrapper to the element. IMO, calling the element setter should
> > do
> > that automatically.
> >
> > On a similar note, Every IUIBase object has a positioner set. I don’t
> > know
> > of a single class which has a different positioner than the element.
> > It
> > seems to me that positioner should be a getter (which normally
> returns
> > the
> > element) that’s overridden for classes which need a different one.
> > That
> > will save memory for every IUIBase created.
> >
> > Harbs
> >
> >> On Sep 26, 2017, at 3:23 PM, Peter Ent 
> >> wrote:
> >>
> >> The setter for element is in HTMLElementWrapper, the super class for
> >> UIBase. The setter for flexes_wrapper is in UIBase. So if the
> element
> >> setter were to also set the flexjs_wrapper, it would have to be an
> >> override in UIBase to do it. At least that¹s how I understand it.
> >>
> >> Could you elaborate a little more on the issue that is raising this
> >> concern?
> >>
> >> Your question made me scan through these classes. Looking at this
> >> code
> > now
> >> makes me think we can do a better and more consistent job organizing

Re: FlexJS element setter

[Peter Ent]

@Harbs: yes on get positioner returning element. This way someone could
override the getter and return something else if it suited their needs.

—peter

On 9/26/17, 9:25 AM, "Harbs"  wrote:

>I looked at MDL and I don’t see any problem there.
>
>I’m talking about simplifying things across the board. I don’t see how it
>could effect anything.
>
>@Peter I think removing positioner might not be a bad idea, but keeping
>it and using it as a pointer to element is basically just as cheap.
>
>> On Sep 26, 2017, at 4:12 PM, Piotr Zarzycki 
>>wrote:
>> 
>> Hi Harbs,
>> 
>> If you will do such changes like moving to set flexjs_wrapper in the
>>setter
>> of element - please make it on the separate branch. Let me test with my
>>app
>> whether MDL will not breaking up. I hope that we could avoid this one,
>>even
>> if I think that it seems to be quite reasonable to do that.
>> 
>> Can you for example do this only for your custom component not for the
>> global IUIBase class ?
>> 
>> Let see what Peter say.
>> 
>> Thanks, Piotr
>> 
>> 
>> 2017-09-26 15:02 GMT+02:00 Harbs :
>> 
>>> Yishay and I were working on drag/drop today and we were modifying one
>>>of
>>> the classes you wrote for generating the drag image.
>>> 
>>> The code can be simplified by using cloneNode() and stuffing the
>>>results
>>> into the element. The thing is, it does not work until you assign the
>>> flexjs_wrapper to the element. IMO, calling the element setter should
>>>do
>>> that automatically.
>>> 
>>> On a similar note, Every IUIBase object has a positioner set. I don’t
>>>know
>>> of a single class which has a different positioner than the element. It
>>> seems to me that positioner should be a getter (which normally returns
>>>the
>>> element) that’s overridden for classes which need a different one. That
>>> will save memory for every IUIBase created.
>>> 
>>> Harbs
>>> 
 On Sep 26, 2017, at 3:23 PM, Peter Ent  wrote:
 
 The setter for element is in HTMLElementWrapper, the super class for
 UIBase. The setter for flexes_wrapper is in UIBase. So if the element
 setter were to also set the flexjs_wrapper, it would have to be an
 override in UIBase to do it. At least that¹s how I understand it.
 
 Could you elaborate a little more on the issue that is raising this
 concern?
 
 Your question made me scan through these classes. Looking at this code
>>> now
 makes me think we can do a better and more consistent job organizing
it
 for Royale. After all, having code that can be quickly understood and
 modified is important.
 
 ‹peter
 
 On 9/26/17, 7:13 AM, "Harbs"  wrote:
 
> Currently, setting the element of a IUIBase will not work correctly
> because the flexjs_wrapper is not set. This makes it error prone when
> there¹s a need to work with the underlying DOM elements for HTML
>output.
> 
> I cannot think of a reason why the wrapper should not be set when
>>> calling
> the element setter. Instead of setting the flexjs_wrapper in
> createElement(), it seems to me that it should be set in the element
> setter in HTMLElementWrapper.
> 
> Anyone have a reason not to do this?
> 
> Harbs
 
>>> 
>>> 
>> 
>> 
>> -- 
>> 
>> Piotr Zarzycki
>> 
>> mobile: +48 880 859 557
>> skype: zarzycki10
>> 
>> LinkedIn: 
>>https://na01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.linke
>>din.com%2Fpiotrzarzycki=02%7C01%7C%7C6716901213624a0e804708d504e2039
>>f%7Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C0%7C636420291136295544=f
>>Q1CjLGkCpNKxSQBmZn%2BnKZEplQpGl25XACOqq0gO2o%3D=0
>> 
>>>edin.com%2Fin%2Fpiotr-zarzycki-92a53552=02%7C01%7C%7C6716901213624a0
>>e804708d504e2039f%7Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C0%7C6364202911
>>36295544=LzIej2n6WVnm9AG1Hi4NqIZjOQS%2Byo4w%2BPYTX0Rq8Gc%3D
>>d=0>
>> 
>> GitHub: 
>>https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.c
>>om%2Fpiotrzarzycki21=02%7C01%7C%7C6716901213624a0e804708d504e2039f%7
>>Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C0%7C636420291136295544=WeIl
>>LzVsJzRKniD1r9F2xb%2BwljhCLHnurBnl03kBM9E%3D=0
>

Re: FlexJS element setter

[Harbs]

I looked at MDL and I don’t see any problem there.

I’m talking about simplifying things across the board. I don’t see how it could 
effect anything.

@Peter I think removing positioner might not be a bad idea, but keeping it and 
using it as a pointer to element is basically just as cheap.

> On Sep 26, 2017, at 4:12 PM, Piotr Zarzycki  wrote:
> 
> Hi Harbs,
> 
> If you will do such changes like moving to set flexjs_wrapper in the setter
> of element - please make it on the separate branch. Let me test with my app
> whether MDL will not breaking up. I hope that we could avoid this one, even
> if I think that it seems to be quite reasonable to do that.
> 
> Can you for example do this only for your custom component not for the
> global IUIBase class ?
> 
> Let see what Peter say.
> 
> Thanks, Piotr
> 
> 
> 2017-09-26 15:02 GMT+02:00 Harbs :
> 
>> Yishay and I were working on drag/drop today and we were modifying one of
>> the classes you wrote for generating the drag image.
>> 
>> The code can be simplified by using cloneNode() and stuffing the results
>> into the element. The thing is, it does not work until you assign the
>> flexjs_wrapper to the element. IMO, calling the element setter should do
>> that automatically.
>> 
>> On a similar note, Every IUIBase object has a positioner set. I don’t know
>> of a single class which has a different positioner than the element. It
>> seems to me that positioner should be a getter (which normally returns the
>> element) that’s overridden for classes which need a different one. That
>> will save memory for every IUIBase created.
>> 
>> Harbs
>> 
>>> On Sep 26, 2017, at 3:23 PM, Peter Ent  wrote:
>>> 
>>> The setter for element is in HTMLElementWrapper, the super class for
>>> UIBase. The setter for flexes_wrapper is in UIBase. So if the element
>>> setter were to also set the flexjs_wrapper, it would have to be an
>>> override in UIBase to do it. At least that¹s how I understand it.
>>> 
>>> Could you elaborate a little more on the issue that is raising this
>>> concern?
>>> 
>>> Your question made me scan through these classes. Looking at this code
>> now
>>> makes me think we can do a better and more consistent job organizing it
>>> for Royale. After all, having code that can be quickly understood and
>>> modified is important.
>>> 
>>> ‹peter
>>> 
>>> On 9/26/17, 7:13 AM, "Harbs"  wrote:
>>> 
 Currently, setting the element of a IUIBase will not work correctly
 because the flexjs_wrapper is not set. This makes it error prone when
 there¹s a need to work with the underlying DOM elements for HTML output.
 
 I cannot think of a reason why the wrapper should not be set when
>> calling
 the element setter. Instead of setting the flexjs_wrapper in
 createElement(), it seems to me that it should be set in the element
 setter in HTMLElementWrapper.
 
 Anyone have a reason not to do this?
 
 Harbs
>>> 
>> 
>> 
> 
> 
> -- 
> 
> Piotr Zarzycki
> 
> mobile: +48 880 859 557
> skype: zarzycki10
> 
> LinkedIn: http://www.linkedin.com/piotrzarzycki
> 
> 
> GitHub: https://github.com/piotrzarzycki21

Re: FlexJS element setter

[Peter Ent]

The original intent of positioner was to allow the element to be
different. The NumericStepper was the use case. The positioner was to be
the enclosing div or DisplayObjectContainer. The element was to be the
input or TextField. We thought there might be more complex components that
would benefit from that treatment.

The problem is, I think most developers would access element for
everything and simply forget the positioner. Also, element and positioner
should really be hidden from app developers are they are platform
dependent (or should be IMO). So a component like NumericStepper might let
you do: stepper.inputField by that itself is a FlexJS component,
TextInput, so that would be safe. Likewise for the buttons.

At this point, I'm not sure positioner is valuable enough to even keep.
And element/flexjs_wrapper are closely paired.

—peter

On 9/26/17, 9:02 AM, "Harbs"  wrote:

>Yishay and I were working on drag/drop today and we were modifying one of
>the classes you wrote for generating the drag image.
>
>The code can be simplified by using cloneNode() and stuffing the results
>into the element. The thing is, it does not work until you assign the
>flexjs_wrapper to the element. IMO, calling the element setter should do
>that automatically.
>
>On a similar note, Every IUIBase object has a positioner set. I don’t
>know of a single class which has a different positioner than the element.
>It seems to me that positioner should be a getter (which normally returns
>the element) that’s overridden for classes which need a different one.
>That will save memory for every IUIBase created.
>
>Harbs
>
>> On Sep 26, 2017, at 3:23 PM, Peter Ent  wrote:
>> 
>> The setter for element is in HTMLElementWrapper, the super class for
>> UIBase. The setter for flexes_wrapper is in UIBase. So if the element
>> setter were to also set the flexjs_wrapper, it would have to be an
>> override in UIBase to do it. At least that¹s how I understand it.
>> 
>> Could you elaborate a little more on the issue that is raising this
>> concern?   
>> 
>> Your question made me scan through these classes. Looking at this code
>>now
>> makes me think we can do a better and more consistent job organizing it
>> for Royale. After all, having code that can be quickly understood and
>> modified is important.
>> 
>> ‹peter
>> 
>> On 9/26/17, 7:13 AM, "Harbs"  wrote:
>> 
>>> Currently, setting the element of a IUIBase will not work correctly
>>> because the flexjs_wrapper is not set. This makes it error prone when
>>> there¹s a need to work with the underlying DOM elements for HTML
>>>output.
>>> 
>>> I cannot think of a reason why the wrapper should not be set when
>>>calling
>>> the element setter. Instead of setting the flexjs_wrapper in
>>> createElement(), it seems to me that it should be set in the element
>>> setter in HTMLElementWrapper.
>>> 
>>> Anyone have a reason not to do this?
>>> 
>>> Harbs
>> 
>

Re: FlexJS element setter

[Piotr Zarzycki]

Hi Harbs,

If you will do such changes like moving to set flexjs_wrapper in the setter
of element - please make it on the separate branch. Let me test with my app
whether MDL will not breaking up. I hope that we could avoid this one, even
if I think that it seems to be quite reasonable to do that.

Can you for example do this only for your custom component not for the
global IUIBase class ?

Let see what Peter say.

Thanks, Piotr


2017-09-26 15:02 GMT+02:00 Harbs :

> Yishay and I were working on drag/drop today and we were modifying one of
> the classes you wrote for generating the drag image.
>
> The code can be simplified by using cloneNode() and stuffing the results
> into the element. The thing is, it does not work until you assign the
> flexjs_wrapper to the element. IMO, calling the element setter should do
> that automatically.
>
> On a similar note, Every IUIBase object has a positioner set. I don’t know
> of a single class which has a different positioner than the element. It
> seems to me that positioner should be a getter (which normally returns the
> element) that’s overridden for classes which need a different one. That
> will save memory for every IUIBase created.
>
> Harbs
>
> > On Sep 26, 2017, at 3:23 PM, Peter Ent  wrote:
> >
> > The setter for element is in HTMLElementWrapper, the super class for
> > UIBase. The setter for flexes_wrapper is in UIBase. So if the element
> > setter were to also set the flexjs_wrapper, it would have to be an
> > override in UIBase to do it. At least that¹s how I understand it.
> >
> > Could you elaborate a little more on the issue that is raising this
> > concern?
> >
> > Your question made me scan through these classes. Looking at this code
> now
> > makes me think we can do a better and more consistent job organizing it
> > for Royale. After all, having code that can be quickly understood and
> > modified is important.
> >
> > ‹peter
> >
> > On 9/26/17, 7:13 AM, "Harbs"  wrote:
> >
> >> Currently, setting the element of a IUIBase will not work correctly
> >> because the flexjs_wrapper is not set. This makes it error prone when
> >> there¹s a need to work with the underlying DOM elements for HTML output.
> >>
> >> I cannot think of a reason why the wrapper should not be set when
> calling
> >> the element setter. Instead of setting the flexjs_wrapper in
> >> createElement(), it seems to me that it should be set in the element
> >> setter in HTMLElementWrapper.
> >>
> >> Anyone have a reason not to do this?
> >>
> >> Harbs
> >
>
>


-- 

Piotr Zarzycki

mobile: +48 880 859 557
skype: zarzycki10

LinkedIn: http://www.linkedin.com/piotrzarzycki


GitHub: https://github.com/piotrzarzycki21

Re: FlexJS element setter

[Harbs]

Yishay and I were working on drag/drop today and we were modifying one of the 
classes you wrote for generating the drag image.

The code can be simplified by using cloneNode() and stuffing the results into 
the element. The thing is, it does not work until you assign the flexjs_wrapper 
to the element. IMO, calling the element setter should do that automatically.

On a similar note, Every IUIBase object has a positioner set. I don’t know of a 
single class which has a different positioner than the element. It seems to me 
that positioner should be a getter (which normally returns the element) that’s 
overridden for classes which need a different one. That will save memory for 
every IUIBase created.

Harbs

> On Sep 26, 2017, at 3:23 PM, Peter Ent  wrote:
> 
> The setter for element is in HTMLElementWrapper, the super class for
> UIBase. The setter for flexes_wrapper is in UIBase. So if the element
> setter were to also set the flexjs_wrapper, it would have to be an
> override in UIBase to do it. At least that¹s how I understand it.
> 
> Could you elaborate a little more on the issue that is raising this
> concern?   
> 
> Your question made me scan through these classes. Looking at this code now
> makes me think we can do a better and more consistent job organizing it
> for Royale. After all, having code that can be quickly understood and
> modified is important.
> 
> ‹peter
> 
> On 9/26/17, 7:13 AM, "Harbs"  wrote:
> 
>> Currently, setting the element of a IUIBase will not work correctly
>> because the flexjs_wrapper is not set. This makes it error prone when
>> there¹s a need to work with the underlying DOM elements for HTML output.
>> 
>> I cannot think of a reason why the wrapper should not be set when calling
>> the element setter. Instead of setting the flexjs_wrapper in
>> createElement(), it seems to me that it should be set in the element
>> setter in HTMLElementWrapper.
>> 
>> Anyone have a reason not to do this?
>> 
>> Harbs
> 

Re: FlexJS element setter

[Peter Ent]

The setter for element is in HTMLElementWrapper, the super class for
UIBase. The setter for flexes_wrapper is in UIBase. So if the element
setter were to also set the flexjs_wrapper, it would have to be an
override in UIBase to do it. At least that¹s how I understand it.

Could you elaborate a little more on the issue that is raising this
concern?   

Your question made me scan through these classes. Looking at this code now
makes me think we can do a better and more consistent job organizing it
for Royale. After all, having code that can be quickly understood and
modified is important.

‹peter

On 9/26/17, 7:13 AM, "Harbs"  wrote:

>Currently, setting the element of a IUIBase will not work correctly
>because the flexjs_wrapper is not set. This makes it error prone when
>there¹s a need to work with the underlying DOM elements for HTML output.
>
>I cannot think of a reason why the wrapper should not be set when calling
>the element setter. Instead of setting the flexjs_wrapper in
>createElement(), it seems to me that it should be set in the element
>setter in HTMLElementWrapper.
>
>Anyone have a reason not to do this?
>
>Harbs