[Trinidad] panelPopup in 1.0.3 SNAPSHOT
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
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
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
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
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