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" <harbs.li...@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 <aha...@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" <harbs.li...@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 <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.
>>>>>>>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
>>>>>>
>>>>>
>>>>
>>>
>>
>
