You are viewing a plain text version of this content. The canonical link for it is here.

Posted to dev@flex.apache.org by Harbs <ha...@gmail.com> on 2017/09/26 11:13:05 UTC

FlexJS element setter

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

Posted by Peter Ent <pe...@adobe.com.INVALID>.

I should also have addressed "cloneNode". I keep operating in the twilight
world between SWF and HTML/JS. The main guiding principle is to make it
work for HTML/JS as simply as possible and then replicate it for SWF. I
think since the Flash announcement, this should really be the defining
principle of RoyaleJS. If we have to make the SWF side pay a bit more,
then so be it. Again, my opinion.

So if cloneNode is more helpful, then do that.

—peter

On 9/26/17, 9:02 AM, "Harbs" <ha...@gmail.com> 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 <pe...@adobe.com.INVALID> 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" <ha...@gmail.com> 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

Posted by Harbs <ha...@gmail.com>.

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 <ah...@adobe.com.INVALID> 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" <ha...@gmail.com> 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 <ah...@adobe.com.INVALID>
>>> 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" <ha...@gmail.com> 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 <ah...@adobe.com.INVALID>
>>>>> 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" <pe...@adobe.com.INVALID> 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" <ha...@gmail.com> 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
>>>>>>>> <pi...@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 <ha...@gmail.com>:
>>>>>>>> 
>>>>>>>>> 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 <pe...@adobe.com.INVALID>
>>>>>>>>>> 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" <ha...@gmail.com> 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.
>>>>>>>> li
>>>>>>>> nk
>>>>>>>> e
>>>>>>>> 
>>>>>>>> 
>>>>>>>> din.com%2Fpiotrzarzycki&data=02%7C01%7C%7C6716901213624a0e804708d504
>>>>>>>> e2
>>>>>>>> 03
>>>>>>>> 9
>>>>>>>> 
>>>>>>>> 
>>>>>>>> f%7Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C0%7C636420291136295544&sd
>>>>>>>> at
>>>>>>>> a=
>>>>>>>> f
>>>>>>>> Q1CjLGkCpNKxSQBmZn%2BnKZEplQpGl25XACOqq0gO2o%3D&reserved=0
>>>>>>>> 
>>>>>>>> 
>>>>>>>> 
>>>>>>>> <https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpl
>>>>>>>> .l
>>>>>>>> in
>>>>>>>> k
>>>>>>>> 
>>>>>>>> 
>>>>>>>> edin.com%2Fin%2Fpiotr-zarzycki-92a53552&data=02%7C01%7C%7C6716901213
>>>>>>>> 62
>>>>>>>> 4a
>>>>>>>> 0
>>>>>>>> 
>>>>>>>> 
>>>>>>>> e804708d504e2039f%7Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C0%7C63642
>>>>>>>> 02
>>>>>>>> 91
>>>>>>>> 1
>>>>>>>> 
>>>>>>>> 
>>>>>>>> 36295544&sdata=LzIej2n6WVnm9AG1Hi4NqIZjOQS%2Byo4w%2BPYTX0Rq8Gc%3D&re
>>>>>>>> se
>>>>>>>> rv
>>>>>>>> e
>>>>>>>> d=0>
>>>>>>>> 
>>>>>>>> GitHub: 
>>>>>>>> 
>>>>>>>> 
>>>>>>>> https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit
>>>>>>>> hu
>>>>>>>> b.
>>>>>>>> c
>>>>>>>> 
>>>>>>>> 
>>>>>>>> om%2Fpiotrzarzycki21&data=02%7C01%7C%7C6716901213624a0e804708d504e20
>>>>>>>> 39
>>>>>>>> f%
>>>>>>>> 7
>>>>>>>> 
>>>>>>>> 
>>>>>>>> Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C0%7C636420291136295544&sdata
>>>>>>>> =W
>>>>>>>> eI
>>>>>>>> l
>>>>>>>> LzVsJzRKniD1r9F2xb%2BwljhCLHnurBnl03kBM9E%3D&reserved=0
>>>>>>> 
>>>>>> 
>>>>> 
>>>> 
>>> 
>> 
>

Re: FlexJS element setter

Posted by Harbs <ha...@gmail.com>.

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 <ah...@adobe.com.INVALID> 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" <ha...@gmail.com> wrote:
> 
>> I’ve used it.
>> 
>>> On Sep 26, 2017, at 8:41 PM, Alex Harui <ah...@adobe.com.INVALID>
>>> wrote:
>>> 
>>> IMO, the first question is, do we really need to support "new UIBase()"?
>> 
>

Re: FlexJS element setter

Posted by Alex Harui <ah...@adobe.com.INVALID>.

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" <ha...@gmail.com> wrote:

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

Re: FlexJS element setter

Posted by Harbs <ha...@gmail.com>.

I’ve used it.

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

Re: FlexJS element setter

Posted by Alex Harui <ah...@adobe.com.INVALID>.

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" <ha...@gmail.com> 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 <ah...@adobe.com.INVALID>
>>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" <ha...@gmail.com> 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 <ah...@adobe.com.INVALID>
>>>> 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" <pe...@adobe.com.INVALID> 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" <ha...@gmail.com> 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
>>>>>>> <pi...@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 <ha...@gmail.com>:
>>>>>>> 
>>>>>>>> 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 <pe...@adobe.com.INVALID>
>>>>>>>>> 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" <ha...@gmail.com> 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.
>>>>>>>li
>>>>>>> nk
>>>>>>> e
>>>>>>> 
>>>>>>> 
>>>>>>>din.com%2Fpiotrzarzycki&data=02%7C01%7C%7C6716901213624a0e804708d504
>>>>>>>e2
>>>>>>> 03
>>>>>>> 9
>>>>>>> 
>>>>>>> 
>>>>>>>f%7Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C0%7C636420291136295544&sd
>>>>>>>at
>>>>>>> a=
>>>>>>> f
>>>>>>> Q1CjLGkCpNKxSQBmZn%2BnKZEplQpGl25XACOqq0gO2o%3D&reserved=0
>>>>>>> 
>>>>>>> 
>>>>>>> 
>>>>>>><https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpl
>>>>>>>.l
>>>>>>> in
>>>>>>> k
>>>>>>> 
>>>>>>> 
>>>>>>>edin.com%2Fin%2Fpiotr-zarzycki-92a53552&data=02%7C01%7C%7C6716901213
>>>>>>>62
>>>>>>> 4a
>>>>>>> 0
>>>>>>> 
>>>>>>> 
>>>>>>>e804708d504e2039f%7Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C0%7C63642
>>>>>>>02
>>>>>>> 91
>>>>>>> 1
>>>>>>> 
>>>>>>> 
>>>>>>>36295544&sdata=LzIej2n6WVnm9AG1Hi4NqIZjOQS%2Byo4w%2BPYTX0Rq8Gc%3D&re
>>>>>>>se
>>>>>>> rv
>>>>>>> e
>>>>>>> d=0>
>>>>>>> 
>>>>>>> GitHub: 
>>>>>>> 
>>>>>>> 
>>>>>>>https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit
>>>>>>>hu
>>>>>>> b.
>>>>>>> c
>>>>>>> 
>>>>>>> 
>>>>>>>om%2Fpiotrzarzycki21&data=02%7C01%7C%7C6716901213624a0e804708d504e20
>>>>>>>39
>>>>>>> f%
>>>>>>> 7
>>>>>>> 
>>>>>>> 
>>>>>>>Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C0%7C636420291136295544&sdata
>>>>>>>=W
>>>>>>> eI
>>>>>>> l
>>>>>>> LzVsJzRKniD1r9F2xb%2BwljhCLHnurBnl03kBM9E%3D&reserved=0
>>>>>> 
>>>>> 
>>>> 
>>> 
>> 
>

Re: FlexJS element setter

Posted by Harbs <ha...@gmail.com>.

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 <ah...@adobe.com.INVALID> 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" <ha...@gmail.com> 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 <ah...@adobe.com.INVALID>
>>> 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" <pe...@adobe.com.INVALID> 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" <ha...@gmail.com> 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
>>>>>> <pi...@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 <ha...@gmail.com>:
>>>>>> 
>>>>>>> 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 <pe...@adobe.com.INVALID>
>>>>>>>> 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" <ha...@gmail.com> 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.li
>>>>>> nk
>>>>>> e
>>>>>> 
>>>>>> din.com%2Fpiotrzarzycki&data=02%7C01%7C%7C6716901213624a0e804708d504e2
>>>>>> 03
>>>>>> 9
>>>>>> 
>>>>>> f%7Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C0%7C636420291136295544&sdat
>>>>>> a=
>>>>>> f
>>>>>> Q1CjLGkCpNKxSQBmZn%2BnKZEplQpGl25XACOqq0gO2o%3D&reserved=0
>>>>>> 
>>>>>> 
>>>>>> <https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpl.l
>>>>>> in
>>>>>> k
>>>>>> 
>>>>>> edin.com%2Fin%2Fpiotr-zarzycki-92a53552&data=02%7C01%7C%7C671690121362
>>>>>> 4a
>>>>>> 0
>>>>>> 
>>>>>> e804708d504e2039f%7Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C0%7C6364202
>>>>>> 91
>>>>>> 1
>>>>>> 
>>>>>> 36295544&sdata=LzIej2n6WVnm9AG1Hi4NqIZjOQS%2Byo4w%2BPYTX0Rq8Gc%3D&rese
>>>>>> rv
>>>>>> e
>>>>>> d=0>
>>>>>> 
>>>>>> GitHub: 
>>>>>> 
>>>>>> https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithu
>>>>>> b.
>>>>>> c
>>>>>> 
>>>>>> om%2Fpiotrzarzycki21&data=02%7C01%7C%7C6716901213624a0e804708d504e2039
>>>>>> f%
>>>>>> 7
>>>>>> 
>>>>>> Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C0%7C636420291136295544&sdata=W
>>>>>> eI
>>>>>> l
>>>>>> LzVsJzRKniD1r9F2xb%2BwljhCLHnurBnl03kBM9E%3D&reserved=0
>>>>> 
>>>> 
>>> 
>> 
>

Re: FlexJS element setter

Posted by Alex Harui <ah...@adobe.com.INVALID>.

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" <ha...@gmail.com> 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 <ah...@adobe.com.INVALID>
>>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" <pe...@adobe.com.INVALID> 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" <ha...@gmail.com> 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
>>>>><pi...@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 <ha...@gmail.com>:
>>>>> 
>>>>>> 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 <pe...@adobe.com.INVALID>
>>>>>>> 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" <ha...@gmail.com> 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.li
>>>>>nk
>>>>> e
>>>>> 
>>>>>din.com%2Fpiotrzarzycki&data=02%7C01%7C%7C6716901213624a0e804708d504e2
>>>>>03
>>>>> 9
>>>>> 
>>>>>f%7Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C0%7C636420291136295544&sdat
>>>>>a=
>>>>> f
>>>>> Q1CjLGkCpNKxSQBmZn%2BnKZEplQpGl25XACOqq0gO2o%3D&reserved=0
>>>>> 
>>>>> 
>>>>><https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpl.l
>>>>>in
>>>>> k
>>>>> 
>>>>>edin.com%2Fin%2Fpiotr-zarzycki-92a53552&data=02%7C01%7C%7C671690121362
>>>>>4a
>>>>> 0
>>>>> 
>>>>>e804708d504e2039f%7Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C0%7C6364202
>>>>>91
>>>>> 1
>>>>> 
>>>>>36295544&sdata=LzIej2n6WVnm9AG1Hi4NqIZjOQS%2Byo4w%2BPYTX0Rq8Gc%3D&rese
>>>>>rv
>>>>> e
>>>>> d=0>
>>>>> 
>>>>> GitHub: 
>>>>> 
>>>>>https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithu
>>>>>b.
>>>>> c
>>>>> 
>>>>>om%2Fpiotrzarzycki21&data=02%7C01%7C%7C6716901213624a0e804708d504e2039
>>>>>f%
>>>>> 7
>>>>> 
>>>>>Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C0%7C636420291136295544&sdata=W
>>>>>eI
>>>>> l
>>>>> LzVsJzRKniD1r9F2xb%2BwljhCLHnurBnl03kBM9E%3D&reserved=0
>>>> 
>>> 
>> 
>

Re: FlexJS element setter

Posted by Harbs <ha...@gmail.com>.

Thanks for looking it over.

Merged.

> On Oct 1, 2017, at 7:07 PM, Piotr Zarzycki <pi...@gmail.com> wrote:
> 
> 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 <piotrzarzycki21@gmail.com <ma...@gmail.com>>:
> 
>> 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 <ha...@gmail.com>:
>> 
>>> I think I’m done. Any reason to not merge into develop?
>>> 
>>>> On Sep 26, 2017, at 7:01 PM, Piotr Zarzycki <pi...@gmail.com>
>>> 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 <ha...@gmail.com>:
>>>> 
>>>>> 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 <ah...@adobe.com.INVALID>
>>>>> 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" <pe...@adobe.com.INVALID> 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" <ha...@gmail.com> 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 <
>>>>> piotrzarzycki21@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 <ha...@gmail.com>:
>>>>>>>>> 
>>>>>>>>>> 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 <pe...@adobe.com.INVALID>
>>>>>>>>>>> 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" <ha...@gmail.com> 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.link
>>>>>>>>> e
>>>>>>>>> din.com%2Fpiotrzarzycki&data=02%7C01%7C%
>>>>> 7C6716901213624a0e804708d504e203
>>>>>>>>> 9
>>>>>>>>> f%7Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C0%
>>>>> 7C636420291136295544&sdata=
>>>>>>>>> f
>>>>>>>>> Q1CjLGkCpNKxSQBmZn%2BnKZEplQpGl25XACOqq0gO2o%3D&reserved=0
>>>>>>>>> 
>>>>>>>>> <https://na01.safelinks.protection.outlook.com/?url=
>>>>> https%3A%2F%2Fpl.lin
>>>>>>>>> k
>>>>>>>>> edin.com%2Fin%2Fpiotr-zarzycki-92a53552&data=02%
>>>>> 7C01%7C%7C6716901213624a
>>>>>>>>> 0
>>>>>>>>> e804708d504e2039f%7Cfa7b1b5a7b34438794aed2c178de
>>>>> cee1%7C0%7C0%7C636420291
>>>>>>>>> 1
>>>>>>>>> 36295544&sdata=LzIej2n6WVnm9AG1Hi4NqIZjOQS%
>>>>> 2Byo4w%2BPYTX0Rq8Gc%3D&reserv
>>>>>>>>> e
>>>>>>>>> d=0>
>>>>>>>>> 
>>>>>>>>> GitHub:
>>>>>>>>> https://na01.safelinks.protection.outlook.com/?url=
>>>>> https%3A%2F%2Fgithub.
>>>>>>>>> c
>>>>>>>>> om%2Fpiotrzarzycki21&data=02%7C01%7C%7C6716901213624a0e80470
>>> 8d504e2
>>>>> 039f%
>>>>>>>>> 7
>>>>>>>>> Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C0%
>>>>> 7C636420291136295544&sdata=WeI
>>>>>>>>> l
>>>>>>>>> LzVsJzRKniD1r9F2xb%2BwljhCLHnurBnl03kBM9E%3D&reserved=0
>>>>>>>> 
>>>>>>> 
>>>>>> 
>>>>> 
>>>>> 
>>>> 
>>>> 
>>>> --
>>>> 
>>>> Piotr Zarzycki
>>>> 
>>>> mobile: +48 880 859 557 <+48%20880%20859%20557>
>>>> skype: zarzycki10
>>>> 
>>>> LinkedIn: http://www.linkedin.com/piotrzarzycki <http://www.linkedin.com/piotrzarzycki>
>>>> <https://pl.linkedin.com/in/piotr-zarzycki-92a53552 <https://pl.linkedin.com/in/piotr-zarzycki-92a53552>>
>>>> 
>>>> GitHub: https://github.com/piotrzarzycki21 <https://github.com/piotrzarzycki21>
>>> 
>>> 
>> 
>> 
>> --
>> 
>> Piotr Zarzycki
>> 
>> mobile: +48 880 859 557 <+48%20880%20859%20557>
>> skype: zarzycki10
>> 
>> LinkedIn: http://www.linkedin.com/piotrzarzycki <http://www.linkedin.com/piotrzarzycki>
>> <https://pl.linkedin.com/in/piotr-zarzycki-92a53552 <https://pl.linkedin.com/in/piotr-zarzycki-92a53552>>
>> 
>> GitHub: https://github.com/piotrzarzycki21 <https://github.com/piotrzarzycki21>
>> 
> 
> 
> 
> -- 
> 
> Piotr Zarzycki
> 
> mobile: +48 880 859 557
> skype: zarzycki10
> 
> LinkedIn: http://www.linkedin.com/piotrzarzycki <http://www.linkedin.com/piotrzarzycki>
> <https://pl.linkedin.com/in/piotr-zarzycki-92a53552 <https://pl.linkedin.com/in/piotr-zarzycki-92a53552>>
> 
> GitHub: https://github.com/piotrzarzycki21 <https://github.com/piotrzarzycki21>

Re: FlexJS element setter

Posted by Piotr Zarzycki <pi...@gmail.com>.

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 <pi...@gmail.com>:

> 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 <ha...@gmail.com>:
>
>> I think I’m done. Any reason to not merge into develop?
>>
>> > On Sep 26, 2017, at 7:01 PM, Piotr Zarzycki <pi...@gmail.com>
>> 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 <ha...@gmail.com>:
>> >
>> >> 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 <ah...@adobe.com.INVALID>
>> >> 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" <pe...@adobe.com.INVALID> 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" <ha...@gmail.com> 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 <
>> >> piotrzarzycki21@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 <ha...@gmail.com>:
>> >>>>>>
>> >>>>>>> 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 <pe...@adobe.com.INVALID>
>> >>>>>>>> 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" <ha...@gmail.com> 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.link
>> >>>>>> e
>> >>>>>> din.com%2Fpiotrzarzycki&data=02%7C01%7C%
>> >> 7C6716901213624a0e804708d504e203
>> >>>>>> 9
>> >>>>>> f%7Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C0%
>> >> 7C636420291136295544&sdata=
>> >>>>>> f
>> >>>>>> Q1CjLGkCpNKxSQBmZn%2BnKZEplQpGl25XACOqq0gO2o%3D&reserved=0
>> >>>>>>
>> >>>>>> <https://na01.safelinks.protection.outlook.com/?url=
>> >> https%3A%2F%2Fpl.lin
>> >>>>>> k
>> >>>>>> edin.com%2Fin%2Fpiotr-zarzycki-92a53552&data=02%
>> >> 7C01%7C%7C6716901213624a
>> >>>>>> 0
>> >>>>>> e804708d504e2039f%7Cfa7b1b5a7b34438794aed2c178de
>> >> cee1%7C0%7C0%7C636420291
>> >>>>>> 1
>> >>>>>> 36295544&sdata=LzIej2n6WVnm9AG1Hi4NqIZjOQS%
>> >> 2Byo4w%2BPYTX0Rq8Gc%3D&reserv
>> >>>>>> e
>> >>>>>> d=0>
>> >>>>>>
>> >>>>>> GitHub:
>> >>>>>> https://na01.safelinks.protection.outlook.com/?url=
>> >> https%3A%2F%2Fgithub.
>> >>>>>> c
>> >>>>>> om%2Fpiotrzarzycki21&data=02%7C01%7C%7C6716901213624a0e80470
>> 8d504e2
>> >> 039f%
>> >>>>>> 7
>> >>>>>> Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C0%
>> >> 7C636420291136295544&sdata=WeI
>> >>>>>> l
>> >>>>>> LzVsJzRKniD1r9F2xb%2BwljhCLHnurBnl03kBM9E%3D&reserved=0
>> >>>>>
>> >>>>
>> >>>
>> >>
>> >>
>> >
>> >
>> > --
>> >
>> > Piotr Zarzycki
>> >
>> > mobile: +48 880 859 557 <+48%20880%20859%20557>
>> > skype: zarzycki10
>> >
>> > LinkedIn: http://www.linkedin.com/piotrzarzycki
>> > <https://pl.linkedin.com/in/piotr-zarzycki-92a53552>
>> >
>> > GitHub: https://github.com/piotrzarzycki21
>>
>>
>
>
> --
>
> Piotr Zarzycki
>
> mobile: +48 880 859 557 <+48%20880%20859%20557>
> skype: zarzycki10
>
> LinkedIn: http://www.linkedin.com/piotrzarzycki
> <https://pl.linkedin.com/in/piotr-zarzycki-92a53552>
>
> GitHub: https://github.com/piotrzarzycki21
>



-- 

Piotr Zarzycki

mobile: +48 880 859 557
skype: zarzycki10

LinkedIn: http://www.linkedin.com/piotrzarzycki
<https://pl.linkedin.com/in/piotr-zarzycki-92a53552>

GitHub: https://github.com/piotrzarzycki21

Re: FlexJS element setter

Posted by Piotr Zarzycki <pi...@gmail.com>.

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 <pi...@gmail.com>:

> 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 <ha...@gmail.com>:
>
>> I think I’m done. Any reason to not merge into develop?
>>
>> > On Sep 26, 2017, at 7:01 PM, Piotr Zarzycki <pi...@gmail.com>
>> 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 <ha...@gmail.com>:
>> >
>> >> 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 <ah...@adobe.com.INVALID>
>> >> 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" <pe...@adobe.com.INVALID> 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" <ha...@gmail.com> 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 <
>> >> piotrzarzycki21@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 <ha...@gmail.com>:
>> >>>>>>
>> >>>>>>> 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 <pe...@adobe.com.INVALID>
>> >>>>>>>> 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" <ha...@gmail.com> 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.link
>> >>>>>> e
>> >>>>>> din.com%2Fpiotrzarzycki&data=02%7C01%7C%
>> >> 7C6716901213624a0e804708d504e203
>> >>>>>> 9
>> >>>>>> f%7Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C0%
>> >> 7C636420291136295544&sdata=
>> >>>>>> f
>> >>>>>> Q1CjLGkCpNKxSQBmZn%2BnKZEplQpGl25XACOqq0gO2o%3D&reserved=0
>> >>>>>>
>> >>>>>> <https://na01.safelinks.protection.outlook.com/?url=
>> >> https%3A%2F%2Fpl.lin
>> >>>>>> k
>> >>>>>> edin.com%2Fin%2Fpiotr-zarzycki-92a53552&data=02%
>> >> 7C01%7C%7C6716901213624a
>> >>>>>> 0
>> >>>>>> e804708d504e2039f%7Cfa7b1b5a7b34438794aed2c178de
>> >> cee1%7C0%7C0%7C636420291
>> >>>>>> 1
>> >>>>>> 36295544&sdata=LzIej2n6WVnm9AG1Hi4NqIZjOQS%
>> >> 2Byo4w%2BPYTX0Rq8Gc%3D&reserv
>> >>>>>> e
>> >>>>>> d=0>
>> >>>>>>
>> >>>>>> GitHub:
>> >>>>>> https://na01.safelinks.protection.outlook.com/?url=
>> >> https%3A%2F%2Fgithub.
>> >>>>>> c
>> >>>>>> om%2Fpiotrzarzycki21&data=02%7C01%7C%7C6716901213624a0e80470
>> 8d504e2
>> >> 039f%
>> >>>>>> 7
>> >>>>>> Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C0%
>> >> 7C636420291136295544&sdata=WeI
>> >>>>>> l
>> >>>>>> LzVsJzRKniD1r9F2xb%2BwljhCLHnurBnl03kBM9E%3D&reserved=0
>> >>>>>
>> >>>>
>> >>>
>> >>
>> >>
>> >
>> >
>> > --
>> >
>> > Piotr Zarzycki
>> >
>> > mobile: +48 880 859 557 <+48%20880%20859%20557>
>> > skype: zarzycki10
>> >
>> > LinkedIn: http://www.linkedin.com/piotrzarzycki
>> > <https://pl.linkedin.com/in/piotr-zarzycki-92a53552>
>> >
>> > GitHub: https://github.com/piotrzarzycki21
>>
>>
>
>
> --
>
> Piotr Zarzycki
>
> mobile: +48 880 859 557 <+48%20880%20859%20557>
> skype: zarzycki10
>
> LinkedIn: http://www.linkedin.com/piotrzarzycki
> <https://pl.linkedin.com/in/piotr-zarzycki-92a53552>
>
> GitHub: https://github.com/piotrzarzycki21
>



-- 

Piotr Zarzycki

mobile: +48 880 859 557
skype: zarzycki10

LinkedIn: http://www.linkedin.com/piotrzarzycki
<https://pl.linkedin.com/in/piotr-zarzycki-92a53552>

GitHub: https://github.com/piotrzarzycki21

Re: FlexJS element setter

Posted by Piotr Zarzycki <pi...@gmail.com>.

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 <ha...@gmail.com>:

> I think I’m done. Any reason to not merge into develop?
>
> > On Sep 26, 2017, at 7:01 PM, Piotr Zarzycki <pi...@gmail.com>
> 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 <ha...@gmail.com>:
> >
> >> 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 <ah...@adobe.com.INVALID>
> >> 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" <pe...@adobe.com.INVALID> 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" <ha...@gmail.com> 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 <
> >> piotrzarzycki21@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 <ha...@gmail.com>:
> >>>>>>
> >>>>>>> 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 <pe...@adobe.com.INVALID>
> >>>>>>>> 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" <ha...@gmail.com> 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.link
> >>>>>> e
> >>>>>> din.com%2Fpiotrzarzycki&data=02%7C01%7C%
> >> 7C6716901213624a0e804708d504e203
> >>>>>> 9
> >>>>>> f%7Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C0%
> >> 7C636420291136295544&sdata=
> >>>>>> f
> >>>>>> Q1CjLGkCpNKxSQBmZn%2BnKZEplQpGl25XACOqq0gO2o%3D&reserved=0
> >>>>>>
> >>>>>> <https://na01.safelinks.protection.outlook.com/?url=
> >> https%3A%2F%2Fpl.lin
> >>>>>> k
> >>>>>> edin.com%2Fin%2Fpiotr-zarzycki-92a53552&data=02%
> >> 7C01%7C%7C6716901213624a
> >>>>>> 0
> >>>>>> e804708d504e2039f%7Cfa7b1b5a7b34438794aed2c178de
> >> cee1%7C0%7C0%7C636420291
> >>>>>> 1
> >>>>>> 36295544&sdata=LzIej2n6WVnm9AG1Hi4NqIZjOQS%
> >> 2Byo4w%2BPYTX0Rq8Gc%3D&reserv
> >>>>>> e
> >>>>>> d=0>
> >>>>>>
> >>>>>> GitHub:
> >>>>>> https://na01.safelinks.protection.outlook.com/?url=
> >> https%3A%2F%2Fgithub.
> >>>>>> c
> >>>>>> om%2Fpiotrzarzycki21&data=02%7C01%7C%7C6716901213624a0e804708d504e2
> >> 039f%
> >>>>>> 7
> >>>>>> Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C0%
> >> 7C636420291136295544&sdata=WeI
> >>>>>> l
> >>>>>> LzVsJzRKniD1r9F2xb%2BwljhCLHnurBnl03kBM9E%3D&reserved=0
> >>>>>
> >>>>
> >>>
> >>
> >>
> >
> >
> > --
> >
> > Piotr Zarzycki
> >
> > mobile: +48 880 859 557
> > skype: zarzycki10
> >
> > LinkedIn: http://www.linkedin.com/piotrzarzycki
> > <https://pl.linkedin.com/in/piotr-zarzycki-92a53552>
> >
> > GitHub: https://github.com/piotrzarzycki21
>
>


-- 

Piotr Zarzycki

mobile: +48 880 859 557
skype: zarzycki10

LinkedIn: http://www.linkedin.com/piotrzarzycki
<https://pl.linkedin.com/in/piotr-zarzycki-92a53552>

GitHub: https://github.com/piotrzarzycki21

Re: FlexJS element setter

Posted by Harbs <ha...@gmail.com>.

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

> On Sep 26, 2017, at 7:01 PM, Piotr Zarzycki <pi...@gmail.com> 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 <ha...@gmail.com>:
> 
>> 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 <ah...@adobe.com.INVALID>
>> 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" <pe...@adobe.com.INVALID> 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" <ha...@gmail.com> 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 <
>> piotrzarzycki21@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 <ha...@gmail.com>:
>>>>>> 
>>>>>>> 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 <pe...@adobe.com.INVALID>
>>>>>>>> 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" <ha...@gmail.com> 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.link
>>>>>> e
>>>>>> din.com%2Fpiotrzarzycki&data=02%7C01%7C%
>> 7C6716901213624a0e804708d504e203
>>>>>> 9
>>>>>> f%7Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C0%
>> 7C636420291136295544&sdata=
>>>>>> f
>>>>>> Q1CjLGkCpNKxSQBmZn%2BnKZEplQpGl25XACOqq0gO2o%3D&reserved=0
>>>>>> 
>>>>>> <https://na01.safelinks.protection.outlook.com/?url=
>> https%3A%2F%2Fpl.lin
>>>>>> k
>>>>>> edin.com%2Fin%2Fpiotr-zarzycki-92a53552&data=02%
>> 7C01%7C%7C6716901213624a
>>>>>> 0
>>>>>> e804708d504e2039f%7Cfa7b1b5a7b34438794aed2c178de
>> cee1%7C0%7C0%7C636420291
>>>>>> 1
>>>>>> 36295544&sdata=LzIej2n6WVnm9AG1Hi4NqIZjOQS%
>> 2Byo4w%2BPYTX0Rq8Gc%3D&reserv
>>>>>> e
>>>>>> d=0>
>>>>>> 
>>>>>> GitHub:
>>>>>> https://na01.safelinks.protection.outlook.com/?url=
>> https%3A%2F%2Fgithub.
>>>>>> c
>>>>>> om%2Fpiotrzarzycki21&data=02%7C01%7C%7C6716901213624a0e804708d504e2
>> 039f%
>>>>>> 7
>>>>>> Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C0%
>> 7C636420291136295544&sdata=WeI
>>>>>> l
>>>>>> LzVsJzRKniD1r9F2xb%2BwljhCLHnurBnl03kBM9E%3D&reserved=0
>>>>> 
>>>> 
>>> 
>> 
>> 
> 
> 
> -- 
> 
> Piotr Zarzycki
> 
> mobile: +48 880 859 557
> skype: zarzycki10
> 
> LinkedIn: http://www.linkedin.com/piotrzarzycki
> <https://pl.linkedin.com/in/piotr-zarzycki-92a53552>
> 
> GitHub: https://github.com/piotrzarzycki21

Re: FlexJS element setter

Posted by Harbs <ha...@gmail.com>.

FYI: TableRowItemRenderer in MDL has a separate positioner.

> On Sep 26, 2017, at 7:01 PM, Piotr Zarzycki <pi...@gmail.com> 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 <ha...@gmail.com>:
> 
>> 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 <ah...@adobe.com.INVALID>
>> 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" <pe...@adobe.com.INVALID> 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" <ha...@gmail.com> 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 <
>> piotrzarzycki21@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 <ha...@gmail.com>:
>>>>>> 
>>>>>>> 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 <pe...@adobe.com.INVALID>
>>>>>>>> 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" <ha...@gmail.com> 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.link
>>>>>> e
>>>>>> din.com%2Fpiotrzarzycki&data=02%7C01%7C%
>> 7C6716901213624a0e804708d504e203
>>>>>> 9
>>>>>> f%7Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C0%
>> 7C636420291136295544&sdata=
>>>>>> f
>>>>>> Q1CjLGkCpNKxSQBmZn%2BnKZEplQpGl25XACOqq0gO2o%3D&reserved=0
>>>>>> 
>>>>>> <https://na01.safelinks.protection.outlook.com/?url=
>> https%3A%2F%2Fpl.lin
>>>>>> k
>>>>>> edin.com%2Fin%2Fpiotr-zarzycki-92a53552&data=02%
>> 7C01%7C%7C6716901213624a
>>>>>> 0
>>>>>> e804708d504e2039f%7Cfa7b1b5a7b34438794aed2c178de
>> cee1%7C0%7C0%7C636420291
>>>>>> 1
>>>>>> 36295544&sdata=LzIej2n6WVnm9AG1Hi4NqIZjOQS%
>> 2Byo4w%2BPYTX0Rq8Gc%3D&reserv
>>>>>> e
>>>>>> d=0>
>>>>>> 
>>>>>> GitHub:
>>>>>> https://na01.safelinks.protection.outlook.com/?url=
>> https%3A%2F%2Fgithub.
>>>>>> c
>>>>>> om%2Fpiotrzarzycki21&data=02%7C01%7C%7C6716901213624a0e804708d504e2
>> 039f%
>>>>>> 7
>>>>>> Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C0%
>> 7C636420291136295544&sdata=WeI
>>>>>> l
>>>>>> LzVsJzRKniD1r9F2xb%2BwljhCLHnurBnl03kBM9E%3D&reserved=0
>>>>> 
>>>> 
>>> 
>> 
>> 
> 
> 
> -- 
> 
> Piotr Zarzycki
> 
> mobile: +48 880 859 557
> skype: zarzycki10
> 
> LinkedIn: http://www.linkedin.com/piotrzarzycki
> <https://pl.linkedin.com/in/piotr-zarzycki-92a53552>
> 
> GitHub: https://github.com/piotrzarzycki21

Re: FlexJS element setter

Posted by Piotr Zarzycki <pi...@gmail.com>.

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 <ha...@gmail.com>:

> 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 <ah...@adobe.com.INVALID>
> 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" <pe...@adobe.com.INVALID> 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" <ha...@gmail.com> 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 <
> piotrzarzycki21@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 <ha...@gmail.com>:
> >>>>
> >>>>> 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 <pe...@adobe.com.INVALID>
> >>>>>> 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" <ha...@gmail.com> 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.link
> >>>> e
> >>>> din.com%2Fpiotrzarzycki&data=02%7C01%7C%
> 7C6716901213624a0e804708d504e203
> >>>> 9
> >>>> f%7Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C0%
> 7C636420291136295544&sdata=
> >>>> f
> >>>> Q1CjLGkCpNKxSQBmZn%2BnKZEplQpGl25XACOqq0gO2o%3D&reserved=0
> >>>>
> >>>> <https://na01.safelinks.protection.outlook.com/?url=
> https%3A%2F%2Fpl.lin
> >>>> k
> >>>> edin.com%2Fin%2Fpiotr-zarzycki-92a53552&data=02%
> 7C01%7C%7C6716901213624a
> >>>> 0
> >>>> e804708d504e2039f%7Cfa7b1b5a7b34438794aed2c178de
> cee1%7C0%7C0%7C636420291
> >>>> 1
> >>>> 36295544&sdata=LzIej2n6WVnm9AG1Hi4NqIZjOQS%
> 2Byo4w%2BPYTX0Rq8Gc%3D&reserv
> >>>> e
> >>>> d=0>
> >>>>
> >>>> GitHub:
> >>>> https://na01.safelinks.protection.outlook.com/?url=
> https%3A%2F%2Fgithub.
> >>>> c
> >>>> om%2Fpiotrzarzycki21&data=02%7C01%7C%7C6716901213624a0e804708d504e2
> 039f%
> >>>> 7
> >>>> Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C0%
> 7C636420291136295544&sdata=WeI
> >>>> l
> >>>> LzVsJzRKniD1r9F2xb%2BwljhCLHnurBnl03kBM9E%3D&reserved=0
> >>>
> >>
> >
>
>


-- 

Piotr Zarzycki

mobile: +48 880 859 557
skype: zarzycki10

LinkedIn: http://www.linkedin.com/piotrzarzycki
<https://pl.linkedin.com/in/piotr-zarzycki-92a53552>

GitHub: https://github.com/piotrzarzycki21

Re: FlexJS element setter

Posted by Harbs <ha...@gmail.com>.

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 <ah...@adobe.com.INVALID> 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" <pe...@adobe.com.INVALID> 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" <ha...@gmail.com> 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 <pi...@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 <ha...@gmail.com>:
>>>> 
>>>>> 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 <pe...@adobe.com.INVALID>
>>>>>> 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" <ha...@gmail.com> 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.link
>>>> e
>>>> din.com%2Fpiotrzarzycki&data=02%7C01%7C%7C6716901213624a0e804708d504e203
>>>> 9
>>>> f%7Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C0%7C636420291136295544&sdata=
>>>> f
>>>> Q1CjLGkCpNKxSQBmZn%2BnKZEplQpGl25XACOqq0gO2o%3D&reserved=0
>>>> 
>>>> <https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpl.lin
>>>> k
>>>> edin.com%2Fin%2Fpiotr-zarzycki-92a53552&data=02%7C01%7C%7C6716901213624a
>>>> 0
>>>> e804708d504e2039f%7Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C0%7C636420291
>>>> 1
>>>> 36295544&sdata=LzIej2n6WVnm9AG1Hi4NqIZjOQS%2Byo4w%2BPYTX0Rq8Gc%3D&reserv
>>>> e
>>>> d=0>
>>>> 
>>>> GitHub: 
>>>> https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.
>>>> c
>>>> om%2Fpiotrzarzycki21&data=02%7C01%7C%7C6716901213624a0e804708d504e2039f%
>>>> 7
>>>> Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C0%7C636420291136295544&sdata=WeI
>>>> l
>>>> LzVsJzRKniD1r9F2xb%2BwljhCLHnurBnl03kBM9E%3D&reserved=0
>>> 
>> 
>

Re: FlexJS element setter

Posted by Alex Harui <ah...@adobe.com.INVALID>.

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" <pe...@adobe.com.INVALID> 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" <ha...@gmail.com> 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 <pi...@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 <ha...@gmail.com>:
>>> 
>>>> 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 <pe...@adobe.com.INVALID>
>>>>>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" <ha...@gmail.com> 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.link
>>>e
>>>din.com%2Fpiotrzarzycki&data=02%7C01%7C%7C6716901213624a0e804708d504e203
>>>9
>>>f%7Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C0%7C636420291136295544&sdata=
>>>f
>>>Q1CjLGkCpNKxSQBmZn%2BnKZEplQpGl25XACOqq0gO2o%3D&reserved=0
>>> 
>>><https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpl.lin
>>>k
>>>edin.com%2Fin%2Fpiotr-zarzycki-92a53552&data=02%7C01%7C%7C6716901213624a
>>>0
>>>e804708d504e2039f%7Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C0%7C636420291
>>>1
>>>36295544&sdata=LzIej2n6WVnm9AG1Hi4NqIZjOQS%2Byo4w%2BPYTX0Rq8Gc%3D&reserv
>>>e
>>>d=0>
>>> 
>>> GitHub: 
>>>https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.
>>>c
>>>om%2Fpiotrzarzycki21&data=02%7C01%7C%7C6716901213624a0e804708d504e2039f%
>>>7
>>>Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C0%7C636420291136295544&sdata=WeI
>>>l
>>>LzVsJzRKniD1r9F2xb%2BwljhCLHnurBnl03kBM9E%3D&reserved=0
>>
>

Re: FlexJS element setter

Posted by Peter Ent <pe...@adobe.com.INVALID>.

@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" <ha...@gmail.com> 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 <pi...@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 <ha...@gmail.com>:
>> 
>>> 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 <pe...@adobe.com.INVALID> 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" <ha...@gmail.com> 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&data=02%7C01%7C%7C6716901213624a0e804708d504e2039
>>f%7Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C0%7C636420291136295544&sdata=f
>>Q1CjLGkCpNKxSQBmZn%2BnKZEplQpGl25XACOqq0gO2o%3D&reserved=0
>> 
>><https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpl.link
>>edin.com%2Fin%2Fpiotr-zarzycki-92a53552&data=02%7C01%7C%7C6716901213624a0
>>e804708d504e2039f%7Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C0%7C6364202911
>>36295544&sdata=LzIej2n6WVnm9AG1Hi4NqIZjOQS%2Byo4w%2BPYTX0Rq8Gc%3D&reserve
>>d=0>
>> 
>> GitHub: 
>>https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.c
>>om%2Fpiotrzarzycki21&data=02%7C01%7C%7C6716901213624a0e804708d504e2039f%7
>>Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C0%7C636420291136295544&sdata=WeIl
>>LzVsJzRKniD1r9F2xb%2BwljhCLHnurBnl03kBM9E%3D&reserved=0
>

Re: FlexJS element setter

Posted by Harbs <ha...@gmail.com>.

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 <pi...@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 <ha...@gmail.com>:
> 
>> 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 <pe...@adobe.com.INVALID> 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" <ha...@gmail.com> 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
> <https://pl.linkedin.com/in/piotr-zarzycki-92a53552>
> 
> GitHub: https://github.com/piotrzarzycki21

Re: FlexJS element setter

Posted by Piotr Zarzycki <pi...@gmail.com>.

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 <ha...@gmail.com>:

> 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 <pe...@adobe.com.INVALID> 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" <ha...@gmail.com> 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
<https://pl.linkedin.com/in/piotr-zarzycki-92a53552>

GitHub: https://github.com/piotrzarzycki21

Re: FlexJS element setter

Posted by Peter Ent <pe...@adobe.com.INVALID>.

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" <ha...@gmail.com> 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 <pe...@adobe.com.INVALID> 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" <ha...@gmail.com> 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

Posted by Harbs <ha...@gmail.com>.

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 <pe...@adobe.com.INVALID> 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" <ha...@gmail.com> 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

Posted by Peter Ent <pe...@adobe.com.INVALID>.

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" <ha...@gmail.com> 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