PDA

View Full Version : Different render modes for accordions



temporary
21 Oct 2008, 7:01 AM
Regarding to this post (link (http://extjs.com/forum/showthread.php?p=240316#post240316)) accordion panels are always rendered with display:none. This has several consequences which make accordion panels very problematic in situations where you have for example combo boxes in an accordion panel.
Please check my first post in the thread mentioned above, there is a simple example which illustrates this effect.

The solve could be to add a render: offsets mode for accordion panels.

Animal
21 Oct 2008, 7:12 AM
It should honour the Components' hideMode

Condor
21 Oct 2008, 7:45 AM
Ext.Panel.collapse() hides the collapseEl element (= bwrap) using standard Ext.Element.visibilityMode (= DISPLAY).

The correct solution would be to support visibilityMode OFFSETS in Ext.Element and assign it to the collapseEl.

A temporary workaround could be:

Ext.Panel.prototype.onRender = Ext.Panel.prototype.onRender.createSequence(function(){
Ext.apply(this[this.collapseEl], {
show: function() {
this.removeClass('x-hidden');
},
hide: function() {
this.addClass('x-hidden');
}
});
});

temporary
21 Oct 2008, 8:31 AM
A temporary workaround could be:

A workaround for me? SCNR ^^

Will this workaround work without further changes? I'll test it later this evening...

jsakalos
21 Oct 2008, 1:04 PM
I would like the idea of (optional) full implementation of lazy instantiating/rendering. Same way as TabPanel is designed.

Condor
21 Oct 2008, 10:57 PM
I would like the idea of (optional) full implementation of lazy instantiating/rendering. Same way as TabPanel is designed.

You would think so, but this has nothing to do with a container rendering its child items.

The root cause is Panel.collapse(), which uses Element.hide() on the bwrap. You would have to adapt Panel.onRender to not render the bwrap when collapsed:true and delay the render of the body until Panel.expand(). And you would have to take special care of config options like el, applyTo, contentEl and html.

All in all, it think it would be easier to just add support for visibilityMode OFFSETS (which IMHO was missing all along) and use this mode on the bwrap in Panel.collapse().

jsakalos
22 Oct 2008, 1:08 AM
That I understand. Anyway, taking it from user space perspective: If I have complex panels in an accordion, all but one collapsed, I do not need them rendered all on page load in majority of cases. It may happend that during the session user never clicks, let's say, 3rd one. The ideal would be to render on first expand (optionally) plus offsets hide mode for cases when I do need collapsed panels rendered, e.g. it's a big form that I want to be submitted.

Condor
22 Oct 2008, 1:18 AM
That would mean that accordion layout should create 'fake' headers for collapsed panels, because the panels themselves wouldn't be rendered yet. That would be a large rewrite for accordion layout!

ps. Even if you change accordion layout, Panel.collapse() should still be fixed, because it can also be used standalone.

jsakalos
22 Oct 2008, 1:37 AM
Yes, that wouldn't be "a-one-line-patch". Something like tabs in TabPanel are done.

Animal
22 Oct 2008, 9:34 AM
It would be nice because then accordions would be able to use BoxComponents as child items. As it is now, it's inconsistent with other layout-managing layout managers in that it only lays out Panels.

temporary
22 Oct 2008, 12:04 PM
Jozefs idea sounds good - but I'd be really happy for a one-line-fix for now. We have a fairly large app with lots of accordions.

mjlecomte
22 Oct 2008, 12:56 PM
Unfortunately I don't really understand what is being discussed here implementation wise, but just wanted to throw in a couple of thoughts should this thread be referenced for any rewrites.

Saki had a version of accordions for Ext1 that supported more than one being open at the same time, a nice feature (It also supported undocking, but that's just icing on the cake).

But, what about accordions implementing something like the north/south regions of border layout where you can expand/collapse them independently, plus the ability to resize them via drag. The only thing missing from border layout I think is the ability to stick in a header even when collapsed that can contain toolbars, tools, etc.

Animal
22 Oct 2008, 11:44 PM
For multiple open at once, I ditched the accordion layout.

I use layout:'anchor', and have all child Panels anchor: '100%', collapsible: true

That, coupled with my layout upgrade which causes container width adjustment on creation and deletion of vertical scrollbars (because expanding >1 accordion item might overflow the height) works well for me.



Ext.override(Ext.layout.ContainerLayout, {
getContainerSize : function(target){
var result = target.dom == document.body ?
target.getViewSize() : target.getStyleSize();
result.width -= target.getPadding('lr');
result.height -= target.getPadding('tb');

if ((target.dom.style.overflow == 'auto') || (target.dom.style.overflowY == 'auto')) {
if (target.dom.scrollHeight > result.height) {
result.width = target.dom.clientWidth;
}
}
return result;
}
});

Ext.override(Ext.layout.AnchorLayout, {
getAnchorViewSize : function(ct, target){
return this.getContainerSize(ct.body||ct.el);
},

// private
adjustWidthAnchor : function(value, comp){
return value - comp.el.getMargins('lr');
},

// private
adjustHeightAnchor : function(value, comp){
return value - comp.el.getMargins('tb');
}
});


Along with the Element.getStyleSize fix:



Ext.override(Ext.Element, {
getStyleSize : function(){
var w, h, d = this.dom, s = d.style;
w = (s.width && s.width != 'auto') ? parseInt(s.width, 10) : this.getWidth();
h = (s.height && s.height != 'auto') ? parseInt(s.height, 10) : this.getHeight();
if(this.isBorderBox()){
h -= this.getBorderWidth('tb');
w -= this.getBorderWidth('lr');
}
return {width: w, height: h};
}
});


No SVN commits from the core devs for weeks now. No progress on enhancements and bugfixes, my ExtOverrides.js grows larger and larger...

jack.slocum
23 Oct 2008, 4:10 AM
Animal, just because nothing has been committed doesn't mean no progress is being made. Keep in mind we always keep the SVN stable, so nothing gets committed until it is ready.

Also, that getStyleSize override could potentially lead to layout issues. It's not a fix - it's a change of the size calculation to not take into account padding, which is non-standard.

Animal
23 Oct 2008, 4:58 AM
Glad to hear things are moving behind the scenes.

Padding should definitely be taken into account when calculating the available area for layouts.

But the version in SVN lumped padding and borders together and only added them "if (Ext.isBorderBox)".

Two errors there. It should use if "(this.isBorderBox())" and the borders should be subtracted if borderBox is set. The padding should be subtracted always.

My ContainerLayout has a getContainerSize which takes padding into account.

I didn't realize that the intention of getStyleSize was to account for both because of the isBorderBox test.

I will fix my code internally, and post it back under the bug report I posted about this.

Animal
23 Oct 2008, 5:00 AM
So, it should be



Ext.override(Ext.Element, {
getStyleSize : function(){
var w, h, d = this.dom, s = d.style;
w = (s.width && s.width != 'auto') ? parseInt(s.width, 10) : this.getWidth();
h = (s.height && s.height != 'auto') ? parseInt(s.height, 10) : this.getHeight();
if(this.isBorderBox()){
h -= this.getBorderWidth('tb');
w -= this.getBorderWidth('lr');
}
return {width: w - this.getPadding('lr'), height: h - this.getPadding('tb')};
}
});


And layouts to use



Ext.override(Ext.layout.ContainerLayout, {
getContainerSize : function(target){
var result = target.dom == document.body ?
target.getViewSize() : target.getStyleSize();

if ((target.dom.style.overflow == 'auto') || (target.dom.style.overflowY == 'auto')) {
if (target.dom.scrollHeight > result.height) {
result.width = target.dom.clientWidth;
}
}
return result;
}
});

jack.slocum
23 Oct 2008, 6:17 AM
When using the style width, the padding and borders have already been calculated in. In border box mode, the padding and borders have not been calculated in and should be added. The current code handles this correctly.

Animal
23 Oct 2008, 6:54 AM
I see, getFrameWidth is intended to provide the total with of both padding and border, taking the borderBox issue into account, and so getStyleSize subtracts that.

So the code can be:



Ext.override(Ext.Element, {
getStyleSize : function(){
var w, h, d = this.dom, s = d.style;
w = (s.width && s.width != 'auto') ? parseInt(s.width, 10) - this.getFrameWidth('lr') : this.getWidth();
h = (s.height && s.height != 'auto') ? parseInt(s.height, 10) - this.getFrameWidth('tb') : this.getHeight();
return {width: w, height: h};
}
});


And getFrameWidth should use this.isBorderBox(), not Ext.isBorderBox. But it looks strange. It returns zero if Ext.isBorderBox, and the sum of padding plus border otherwise. This is the code from SVN:



getFrameWidth : function(sides, onlyContentBox){
return onlyContentBox && Ext.isBorderBox ? 0 : (this.getPadding(sides) + this.getBorderWidth(sides));
},


Shouldn't it be



getFrameWidth : function(sides, onlyContentBox){
return this.getPadding(sides) + (onlyContentBox && this.isBorderBox() ? 0 : this.getBorderWidth(sides));
},


?

jack.slocum
23 Oct 2008, 9:49 AM
Frame width is the width of padding and borders that combined with the style width will add up to the offset width.

Padding is part of the border box quirks.

Standards mode:

content width + padding + borders = offset width

and

style width = content width

However is Border box mode:

style width = offset width

So in border box mode, the frame width (padding + borders) when requesting onlyContentBox mode, should always be zero. Make sense?

I do agree it should be using this.isBorderBox() and it will be changed on next check-in.

jsakalos
23 Oct 2008, 4:38 PM
Well, we stray here from the original lazy instantiating/rendering issue into some padding/margin/size calculation discussion. Let's get back to the original request if possible please

temporary
12 Nov 2008, 1:42 AM
Any news on this one?

Will this be adressed in 3.0?