I absolutely agree about the code redundancy. However at this moment it is
already all over the place. Alert, SkinnablePopUpContainer, PopUpAnchor,
DropDownListBase and perhaps some other classes I'm not thinking of, all
have their own ways of displaying and positioning popups. And - as you
pointed out - none of them offers absolute positioning. To be honest, I
often find PopUpAnchor's positioning rules confusing.
This is why I created 'IPopUpPosition' as an interface for classes that
want to position a popup in a certain way (current implementors are
PopUpCenteredPosition and PopUpAbsolutePosition).
If you want to entirely remove the redundancy around the
creation/destruction of popups, then that logic should also be encapsulated
and reused (something like the existing DropDownController, but in a more
generic way). Of course this touches on so many classes that I didn't dare
venture down that road.
A PopUp tag that can be used inside "declarations" seems like a reasonable
compromise. However I'm unsure as to whether it'll actually solve the
redundancy issue: it'll be yet another class that manages popups as well.
I'd also like to bring a semantic argument to the table:
SkinnablePopUpContainer and PopUpAnchor offer excellent means of popping up
views in a decoupled way. If ever I decide that MyForm, which is currently
displayed through a popup, must be used as a regular view, then I can
easily do so without touching MyForm's code.
However some components will always be popups and are not intended to be
used any other way. Some examples are Alert, Toaster, CallOut, and
DropDownListBase's list. If you are then allowed to write something like:
<fx:Declarations>
<!-- myAlert.open() to open -->
<s:PopUp id="myAlert">
<s:Alert/>
</s:PopUp>
</fx:Declarations>
That's confusing, because Alert *is *a popup. You can not embed it
somewhere in regular view.
<fx:Declarations>
<!-- myAlert.open() to open -->
<s:SkinnablePopUpContainer id="myAlert">
<s:Alert/>
</s:SkinnablePopUpContainer>
</fx:Declarations>
Wouldn't make any sense,because Alert already has a Skin of its own. (This
scenario makes perfect sense for MyForm though, because the form's popup
chrome is decoupled from the actual form)
<fx:Declarations>
<!-- myAlert.open() to open -->
<s:Alert id="myAlert" />
</fx:Declarations>
Conveys the idea that Alert is a component in its own right that is always
used as a popup.
Bottom line: I think there is a place for something like SkinnablePopUp as
a base class, but the actual creation/destruction/positioning behavior
should be encapsulated and reused by other popup components.
M
On Fri, Oct 11, 2013 at 1:13 PM, Maurice Amsellem <
maurice.amsel...@systar.com> wrote:
> I definitely like the idea of declaring Popups in MXML instead of
> ActionScript.
>
> However, there is a downside to your approach, because it induces some
> code redundancy: the popup management (modal, parent, adding/removing to
> PopupManager, centering, etc...)
> must be pushed to the Popup component itself, so that it can be used.
>
> I prefer the more loosely coupled approach used in PopupAnchor, where it's
> the PopupAnchor that manages all the popup behavior (modal, parenting,
> adding/removing to PopupManager, centering, etc...).
> IMO it has the following benefits:
> - no code redundancy, easier maintenance
> - you can popup just anything that is enclosed in PopupAnchor ( Panel,
> Titlewindow, Callout, Toaster, Group, Button...) .
> - you can add new popup behavior (eg. Transitions effects) to PopupAnchor
> and all your popups will benefit from it.
>
> The downsides are the following:
> - it's more verbose: you need two embedded tags instead of one, to
> achieve the same result. That's the price of flexibility
> - the PopupAnchor must be child of it's parent (eg. Button). I would
> prefer it to be independent and be in the <fx:Declarations> section, and
> have an explicit optional "parent" property. That's how Parsley's Popup
> work (see below).
> ------
>
> PS: to say the truth, I am not using PopupAnchor but a variant of it,
> called "Popup" that is part of Cairngorm/Parsley, because it's IOC aware,
> but it's the same idea:
>
> <parsley:PopUp id="createComponentPopup" modal="true"
> open="{model.editDialogPM.showDialog}" paren="{someButton}" >
> <my:MyComponent id="createComponentDlg"/>
> </parsley:PopUp>
>
> What do you think?
>
> Maurice
>
>
> -----Message d'origine-----
> De : Maxime Cowez [mailto:maxime.co...@gmail.com]
> Envoyé : vendredi 11 octobre 2013 12:23
> À : dev@flex.apache.org
> Objet : Re: FLEX-33806 and spark Alert implementation
>
> > What's the difference between you new SkinnablePopup and the existing
> SkinnablePopupContainer ?
>
> It's how SkinnableComponent compares to SkinnableContainer, but for popups.
> So it allows you to create custom popups without having to wrap them in a
> SkinnablePopUpContainer or a PopUpAnchor.
> From a pragmatic point of view it allows you to do things like this:
>
> <fx:Declarations>
> <rs:Alert id="alert" title="Hello world"
> text="Are you sure you wish to say hello?"/>
> </fx:Declarations>
>
> <s:Button label="Say hello" click="alert.open()"/>
>
> As another example implementation of SkinnablePopUp (besides Alert) I've
> created a Toaster class (you know, messages that pop up and stack up in a
> corner of the screen for a few seconds), which can be used like this:
>
> <fx:Declarations>
> <rs:Toaster id="toaster" width="150" bottom="20" right="20"/>
> </fx:Declarations>
>
> <s:Button label="Say hello" click="toaster.toast('Hello there from the
> Toaster')"/>
>
> M
>
>
>
> On Fri, Oct 11, 2013 at 11:34 AM, Maurice Amsellem <
> maurice.amsel...@systar.com> wrote:
>
> > BTW, I like the idea of the mxml based Alert (in addition to AS script
> > API) :-)
> >
> > Maurice
> >
> > -----Message d'origine-----
> > De : Maurice Amsellem [mailto:maurice.amsel...@systar.com]
> > Envoyé : vendredi 11 octobre 2013 11:31 À : dev@flex.apache.org Objet
> > : RE: FLEX-33806 and spark Alert implementation
> >
> > Hi Maxime,
> >
> > What's the difference between you new SkinnablePopup and the existing
> > SkinnablePopupContainer ?
> >
> > Maurice
> >
> > -----Message d'origine-----
> > De : Maxime Cowez [mailto:maxime.co...@gmail.com] Envoyé : vendredi 11
> > octobre 2013 11:25 À : dev@flex.apache.org Objet : Re: FLEX-33806 and
> > spark Alert implementation
> >
> > I've been working on a Spark implementation of Alert, which I've
> > discussed on this list before (see
> > http://apache-flex-development.2333347.n4.nabble.com/DISCUSS-Alerts-an
> > d-dialogs-in-Flex-4-x-Spark-was-Alerts-and-dialogs-in-FlexJS-td27595.h
> > tml
> > ).
> > Unlike the current experimental implementation - which is a port of
> > the old mx Alert - it's rewritten from scratch.
> > I've been meaning to integrate it with the existing implementation so
> > that its API would be more or less backwards compatible and then
> > submit it as a patch, but haven't had a shred of spare time these last
> > few months :(
> >
> > What's a bit different in my approach is that Alert extends
> > SkinnablePopUp (which is a new base class, and hence Alert is no
> > longer a SkinnableContainer). SkinnablePopUp can be extended and/or
> > skinned any which way you want and offers total flexibility, whereas
> > Alert is a default implementation with a few SkinParts.
> > I used these SkinParts:
> >
> > [SkinPart(required="true")]
> > public var textDisplay:IDisplayText;
> >
> > [SkinPart(required="false")]
> > public var titleDisplay:IDisplayText;
> >
> > [SkinPart(required="false")]
> > public var commitButton:IVisualElement;
> >
> > [SkinPart(required="false")]
> > public var discardButton:IVisualElement;
> >
> > [SkinPart(required="false")]
> > public var cancelButton:IVisualElement;
> >
> > [SkinPart(required="false")]
> > public var iconDisplay:BitmapImage;
> >
> > Only 'textDisplay' is required since an Alert without a message
> > doesn't make much sense; the others are optional.
> >
> > Max
> >
> >
> >
> > On Fri, Oct 11, 2013 at 9:34 AM, Maurice Amsellem <
> > maurice.amsel...@systar.com> wrote:
> >
> > > >Simplification tends to remove flexibility. Spark components are
> > supposed
> > > >to leave control of all visuals to the skin designer. For example,
> can
> > > >the custom skin designer swap out a messageDisplay that does plain
> > > >text
> > > with one that does rich text?
> > >
> > > If you replace plain text with rich text, it's not only a skin
> > > design change in my sense, because you are also changing the
> > > behavior (you have to change to calling code as well).
> > > Yet, you can do it by subclassing Alert to RichAlert and just swap
> > > the content and override one of the static methods to use your
> RichText.
> > > With the previous implementation, you could replace the skin part
> > > with RichText in the skin, but you will still have to subclass Alert
> > > to set the richText correctly.
> > >
> > > The problem here is that it's not a Skinnable Component but a
> > > Skinnable Container.
> > > It's different because Skinnable containers content is part on the
> "data"
> > > and not of the skin.
> > > It's not clear to me if changes in the content of
> > > SkinnableContainers should be set as skinParts and moved to the skin
> > > (and disabling the content
> > > skinpart)
> > > or just overriding the content in a subclass of the container.
> > >
> > > What do you think?
> > >
> > > Maurice
> > >
> > > -----Message d'origine-----
> > > De : Alex Harui [mailto:aha...@adobe.com] Envoyé : vendredi 11
> > > octobre
> > > 2013 06:26 À : dev@flex.apache.org Objet : Re: FLEX-33806 and spark
> > > Alert implementation
> > >
> > >
> > >
> > > On 10/10/13 7:20 PM, "Justin Mclean" <jus...@classsoftware.com> wrote:
> > >
> > > >Hi,
> > > >
> > > >Being an experimental component I think just about any improvement
> > > >is a good idea.
> > > I agree with that statement, but is removing skinparts a "good idea"?
> > > Simplification tends to remove flexibility. Spark components are
> > supposed
> > > to leave control of all visuals to the skin designer. For example,
> can
> > > the custom skin designer swap out a messageDisplay that does plain
> > > text with one that does rich text?
> > >
> > > Getting rid of monkey patching PanelSkin sounds like a good idea
> though.
> > > And defaulting to BitmapImage and plain text.
> > >
> > > Have fun,
> > > -Alex
> > >
> > >
> >
>
