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 <aha...@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" <p...@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" <harbs.li...@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 <piotrzarzyck...@gmail.com>
>>>> wrote:
>>>>
>>>> Hi Harbs,
>>>>
>>>> If you will do such changes like moving to set flexjs_wrapper in the
>>>> setter
>>>> of element - please make it on the separate branch. Let me test with my
>>>> app
>>>> whether MDL will not breaking up. I hope that we could avoid this one,
>>>> even
>>>> if I think that it seems to be quite reasonable to do that.
>>>>
>>>> Can you for example do this only for your custom component not for the
>>>> global IUIBase class ?
>>>>
>>>> Let see what Peter say.
>>>>
>>>> Thanks, Piotr
>>>>
>>>>
>>>> 2017-09-26 15:02 GMT+02:00 Harbs <harbs.li...@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 <p...@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" <harbs.li...@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
>>>
>>
>
