flex-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Alex Harui <aha...@adobe.com.INVALID>
Subject Re: FlexJS element setter
Date Tue, 26 Sep 2017 17:41:45 GMT
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.lists@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 <aharui@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.lists@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 <aharui@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" <pent@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.lists@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 <harbs.lists@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 <pent@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.lists@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
>>>>>> 
>>>>> 
>>>> 
>>> 
>> 
>

Mime
View raw message