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