[Trinidad] panelPopup in 1.0.3 SNAPSHOT

2007-09-11 Thread Andrew Robinson
Before filing a bug, I was wondering what the intended result of this was:


test


from what I can see of 1.0.3-SNAPSHOT the "testclass" style class is
ignored. Is it supposed to render on the trigger, the popup content,
the popup outer element or the element that contains the content for
the entire control?

Here is the HTML from the above example:


test



test





Re: [Trinidad] panelPopup in 1.0.3 SNAPSHOT

2007-09-11 Thread Adam Winer
Good question

In general, they go on the root element.  In this case,
that's the .  However, the primary purpose of this
component is definitely the popup itself, , so you could
very reasonably argue that it ought to go there.

Looking at the content, I have more questions:
 - Why do we need three  - one with an ID and
  the visibility:hidden, as well as the "container" and
  "content" style classes?  IMO, we should only need two,
  and the middle one would be unnecessary.
- There's no styleclass on the outer span.

I think what I'd recommend is deleting the middle DIV,
renaming "af|panelPopup::content" to just "af|panelPopup",
rendering styleclass on that content DIV, and finally
adding an af|panelPopup::trigger class onto the outer
span.

Oh, and also doc the styles in the skin-selectors page. :)

-- Adam



On 9/11/07, Andrew Robinson <[EMAIL PROTECTED]> wrote:
> Before filing a bug, I was wondering what the intended result of this was:
>
> 
> test
> 
>
> from what I can see of 1.0.3-SNAPSHOT the "testclass" style class is
> ignored. Is it supposed to render on the trigger, the popup content,
> the popup outer element or the element that contains the content for
> the entire control?
>
> Here is the HTML from the above example:
>
> 
>  onclick="TrPanelPopup.showPopup('_id45_popupContainer', '_id45',
> event, 'click','null',false,0,0,0,0); return false;" name="_id45"
> id="_id45">test
> 
> 
> 
> test
> 
> 
> 
>


Re: [Trinidad] panelPopup in 1.0.3 SNAPSHOT

2007-09-11 Thread Adam Winer
Good question

In general, they go on the root element.  In this case,
that's the .  However, the primary purpose of this
component is definitely the popup itself, , so you could
very reasonably argue that it ought to go there.

Looking at the content, I have more questions:
 - Why do we need three  - one with an ID and
  the visibility:hidden, as well as the "container" and
  "content" style classes?  IMO, we should only need two,
  and the middle one would be unnecessary.
- There's no styleclass on the outer span.

I think what I'd recommend is deleting the middle DIV,
renaming "af|panelPopup::content" to just "af|panelPopup",
rendering styleclass on that content DIV, and finally
adding an af|panelPopup::trigger class onto the outer
span.

Oh, and also doc the styles in the skin-selectors page. :)

-- Adam



On 9/11/07, Andrew Robinson <[EMAIL PROTECTED]> wrote:
> Before filing a bug, I was wondering what the intended result of this was:
>
> 
> test
> 
>
> from what I can see of 1.0.3-SNAPSHOT the "testclass" style class is
> ignored. Is it supposed to render on the trigger, the popup content,
> the popup outer element or the element that contains the content for
> the entire control?
>
> Here is the HTML from the above example:
>
> 
>  onclick="TrPanelPopup.showPopup('_id45_popupContainer', '_id45',
> event, 'click','null',false,0,0,0,0); return false;" name="_id45"
> id="_id45">test
> 
> 
> 
> test
> 
> 
> 
>


Re: [Trinidad] panelPopup in 1.0.3 SNAPSHOT

2007-09-12 Thread Danny Robinson
I noticed the style attributes were not being output as they should a few
days ago.  I've put a fix together, just wanted to test a few other bits.
Certainly triggerStyle was totally unecessary.  I'll make the necessary
tweaks as per Adam's suggestions.

The original reason for 3 divs was to keep seperate the skin styles that a
developer could tweak from those required by the component.  Hence, the
outer div controlled the show/hide and couldn't easily be modified though
bad style entries.

On 9/11/07, Adam Winer <[EMAIL PROTECTED]> wrote:
>
> Good question
>
> In general, they go on the root element.  In this case,
> that's the .  However, the primary purpose of this
> component is definitely the popup itself, , so you could
> very reasonably argue that it ought to go there.
>
> Looking at the content, I have more questions:
> - Why do we need three  - one with an ID and
>   the visibility:hidden, as well as the "container" and
>   "content" style classes?  IMO, we should only need two,
>   and the middle one would be unnecessary.
> - There's no styleclass on the outer span.
>
> I think what I'd recommend is deleting the middle DIV,
> renaming "af|panelPopup::content" to just "af|panelPopup",
> rendering styleclass on that content DIV, and finally
> adding an af|panelPopup::trigger class onto the outer
> span.
>
> Oh, and also doc the styles in the skin-selectors page. :)
>
> -- Adam
>
>
>
> On 9/11/07, Andrew Robinson <[EMAIL PROTECTED]> wrote:
> > Before filing a bug, I was wondering what the intended result of this
> was:
> >
> > 
> > test
> > 
> >
> > from what I can see of 1.0.3-SNAPSHOT the "testclass" style class is
> > ignored. Is it supposed to render on the trigger, the popup content,
> > the popup outer element or the element that contains the content for
> > the entire control?
> >
> > Here is the HTML from the above example:
> >
> > 
> >  > onclick="TrPanelPopup.showPopup('_id45_popupContainer', '_id45',
> > event, 'click','null',false,0,0,0,0); return false;" name="_id45"
> > id="_id45">test
> > 
> > 
> > 
> > test
> > 
> > 
> > 
> >
>



-- 
Chordiant Software Inc.
www.chordiant.com


Re: [Trinidad] panelPopup in 1.0.3 SNAPSHOT

2007-09-12 Thread Adam Winer
On 9/12/07, Danny Robinson <[EMAIL PROTECTED]> wrote:
>
> I noticed the style attributes were not being output as they should a few
> days ago.  I've put a fix together, just wanted to test a few other bits.
> Certainly triggerStyle was totally unecessary.  I'll make the necessary
> tweaks as per Adam's suggestions.
>
> The original reason for 3 divs was to keep seperate the skin styles that a
> developer could tweak from those required by the component.  Hence, the
> outer div controlled the show/hide and couldn't easily be modified though
> bad style entries.



I get the outer div with the show/hide styles.  I don't get why we
need two more divs inside it - I only see a need for one more div
(two total).

-- Adam


On 9/11/07, Adam Winer <[EMAIL PROTECTED]> wrote:
> >
> > Good question
> >
> > In general, they go on the root element.  In this case,
> > that's the .  However, the primary purpose of this
> > component is definitely the popup itself, , so you could
> > very reasonably argue that it ought to go there.
> >
> > Looking at the content, I have more questions:
> > - Why do we need three  - one with an ID and
> >   the visibility:hidden, as well as the "container" and
> >   "content" style classes?  IMO, we should only need two,
> >   and the middle one would be unnecessary.
> > - There's no styleclass on the outer span.
> >
> > I think what I'd recommend is deleting the middle DIV,
> > renaming "af|panelPopup::content" to just "af|panelPopup",
> > rendering styleclass on that content DIV, and finally
> > adding an af|panelPopup::trigger class onto the outer
> > span.
> >
> > Oh, and also doc the styles in the skin-selectors page. :)
> >
> > -- Adam
> >
> >
> >
> > On 9/11/07, Andrew Robinson < [EMAIL PROTECTED]> wrote:
> > > Before filing a bug, I was wondering what the intended result of this
> > was:
> > >
> > > 
> > > test
> > > 
> > >
> > > from what I can see of 1.0.3-SNAPSHOT the "testclass" style class is
> > > ignored. Is it supposed to render on the trigger, the popup content,
> > > the popup outer element or the element that contains the content for
> > > the entire control?
> > >
> > > Here is the HTML from the above example:
> > >
> > > 
> > >  > > onclick=" TrPanelPopup.showPopup('_id45_popupContainer', '_id45',
> > > event, 'click','null',false,0,0,0,0); return false;" name="_id45"
> > > id="_id45">test
> > > 
> > > 
> > > 
> > > test
> > > 
> > > 
> > > 
> > >
> >
>
>
>
> --
> Chordiant Software Inc.
> www.chordiant.com