PDA

View Full Version : BorderLayout fails to layout if no config for region



barrygently
26 Nov 2006, 7:54 PM
If I have a BorderLayout thus:


layout = new YAHOO.ext.BorderLayout (document.body, {
hideOnLayout: true,
west: {
split:true,
initialSize: 200,
minSize: 175,
maxSize: 400,
collapsible: true,
autoScroll: true,
titlebar: true,
collapsible: true
},
center: {
autoScroll:true
}
});
layout.beginUpdate();
layout.add('north', new YAHOO.ext.ContentPanel('menu', {}));
layout.add('west', new YAHOO.ext.ContentPanel('navigation', {}));
layout.add('center', new YAHOO.ext.ContentPanel('main', {closable: false}));
layout.endUpdate ();

It will fail when trying to add the "north" region. To get it to work you have to add an empty config:


north: {}

Of course if I could do:


north: { content: new YAHOO.ext.ContentPanel('menu', {}) }

That would be even better :D

Gary

jack.slocum
27 Nov 2006, 5:08 AM
How are you defining a region with no config and getting the error? I don't see it in your code.

Animal
27 Nov 2006, 6:19 AM
Of course it will fail if you add a ContentPanel to the "north" region, and you have not configured a "north" region!

barrygently
27 Nov 2006, 7:44 AM
Well perhaps there is some confusion on my part, but it's not obvious that the config is there to also define the regions (the documentation doesn't mention that).

In other words, if you are adding to a region that has no config it would be nice then if it "auto" configured that region for you, i.e. added it.

At the moment to define the region you have to add an empty config, which is a little incongruous.

Gary

tryanDLS
27 Nov 2006, 8:30 AM
The purpose of the config is to DEFINE the region. Yeah, the code could guess that you want to auto configure a region that you didn't define, but then next week somebody's going to say,

'I inadvertently added something to a region I didn't define, and it auto-added it - why doesn't it throw an exception?'.

I don't think it's a big deal to specify 'north: {}', then it's obvious looking at that piece of code what the layout is.

barrygently
27 Nov 2006, 8:42 AM
I don't think it's a big deal to specify 'north: {}', then it's obvious looking at that piece of code what the layout is.

That depends upon your point of view.


'I inadvertently added something to a region I didn't define, and it auto-added it - why doesn't it throw an exception?'.

That couldn't happen because if they called layout.add then they will be explicitly specifying the region it gets added to.

As I said, I was thrown off by the name of the parameter and that it doesn't tell you that you have to use the config to define the regions you want to use.

Gary

jack.slocum
27 Nov 2006, 9:45 AM
The config defines the regions you want. Implicitly creating a region (or anything else) is generally bad practice. It makes things difficult to debug. If you want a region, you need to define it.

barrygently
27 Nov 2006, 9:56 AM
Implicitly creating a region (or anything else) is generally bad practice.

I'd disagree there and would point out that you implicitly create tabs when multiple content panels are added to a region.

jack.slocum
27 Nov 2006, 10:48 AM
What I was referring to was implicitly correcting programming mistakes. For example, you don't define a region and then try to add a panel to the undefined region. That's a programmer mistake. The reason for the config is to define what regions you want.

The auto creation of tabs is not implicit, that's the only way it functions with multiple panels. There is no other option.

barrygently
27 Nov 2006, 11:04 AM
Again I'd disagree that it's a programmers mistake because it's making the programmer do the work twice. When they do layout.add they have to explicitly state which region they want to add to. To force them to define a "null" configuration (which isn't documented) for a region when they are explicitly requesting the addition to that region is problematic.

Also consider, as a comparison the java.awt.BorderLayout, you do not have to explicitly "pre-define" which regions you are gonna be working with because the regions are already present.

Remember that the mistake you are wanting to prevent isn't documented, which is why I got tripped up by it. The name of the parameter is "config", indicating configuration NOT specification/definition. Also, by explicitly adding content to a region I know I'm not making a mistake.

jbowman
27 Nov 2006, 11:32 AM
like you said though, in java.awt, the regions already exist. In yui-ext, they don't. Being that we're working in a client/server environment, rather than single machine application, why would we want empty regions predefined to help the developer? Regions and contentpanels have their own configuration options also. This is important for instances where you are creating both regions, and panels empty, then adding to them.

In the examples section, my latest version of the yui-logger in a layout-dialog shows where this comes into play. I had to set initialSize for the regions that I later added contentpanels to. Those contentpanels were empty until I pulled sections of the logger out and placed them in them. Without setting up the region first, including size, the content I appended would not have been displayed.

There's lots of shortcuts offered to application developers for UI development that really just aren't practical in a web development environment, because then not only is cpu and memory being wasted (at insignificant levels) for ease of development, but bandwidth is also. True, any framework comes at the cost of resource usage, but I'm going to have to agree with Jack that the current implementation is probably the better choice for a web environment.

tryanDLS
27 Nov 2006, 11:41 AM
@barrygently:

I think you've made your point that it's not documented. We get it. The doc is a work in progress (notice that it's .33 RC2, not 1.0 gold).

Arguing the semantics of the parm name is a waste of time. Jack has been consistent thru-out the code, naming the object that contains the definition info as 'config'.

If you truly think you're going to save developers time, you can subclass the layout object to create all the regions by default, and do all the necessary coding to handle sizing/scrolling, etc when some of the regions are misssing. If you can justify the time spent to do that vs adding a 'north: null' to your code, go for it. Those of us that have been using this object for longer than you, seem to be in agreement that your way is not the way to go.

barrygently
27 Nov 2006, 4:09 PM
I see, so it's a case of: "shut up, you're new"...

As I suspected... At least I know now...

tryanDLS
27 Nov 2006, 4:27 PM
I'm sorry if you got that impression. I said nothing about you being 'new'. I think everyone here welcomes suggestions, however continuing to harp on things over and over (single points in the doc) becomes counter-productive. If you see an ommission in the doc, please by all means post something about it. Jack is very quick to update the documentation, in response to user posts.

barrygently
27 Nov 2006, 4:55 PM
That's the thing I don't think I am harping over a single point.

Look at it from my perspective, I'm new to yui-ext so I have no idea about the "intentions" of the code, I can only go on the documentation. The intention of the code seems to be that I have to define something twice to make it work, in general I'd consider this is making my life painful for no good reason. Also, it's difficult to understand the intention of the code when it just fails (badly) if you don't configure it correctly, i.e. there is no error message returned when you try to add content to a region that hasn't been configured.

Whilst these are small points (to you and possibly others), for me they are large because it's making my life difficult and I'm having to delve around in the source to try and understand why it's failing. Whenever I've had to do that (in other code I've worked with) it's generally a bug and/or missing documentation.

For example, even if the documentation says that the region must be defined prior to adding content it's still a problem that this error condition isn't trapped if the developer doesn't add the configuration.

Now the ultimate reason for "nit-picking" here is to make (in my mind) yui-ext better, that was the purpose of the bug report. But to be told that "you're just doing the wrong thing" seems a litte hostile. I could easily have just overridden the offending function layout.add with no problem but I wanted to report this to try and make things "better".

jack.slocum
27 Nov 2006, 5:38 PM
Barry,

My original response was "The config defines the regions you want. Implicitly creating a region (or anything else) is generally bad practice. It makes things difficult to debug. If you want a region, you need to define it.". There was no hostility intended.

I think the debate started over whether or not you should have to define the regions. You believe it should be one way, while myself and the others who have commented think how it currently works is preferred/correct.

I understand you are new to yui-ext and I appreciate the feedback. One thing I would recommend is taking the time to become familiar with the library more before being "forceful" with suggestions. The library is pretty consistent, and I think that is important. If something seems verbose, there's generally a good reason. I don't like to write extra code more than anyone else.

"The intention of the code seems to be that I have to define something twice to make it work, in general I'd consider this is making my life painful for no good reason"

There's no defining twice. You are telling the layout where you want your panel to go. That has nothing to do with defining the regions. In fact, the function you are using is just a convenience function for:

var north = layout.getRegion('north');
north.add(yourPanel);

The "config" is in fact a config - for the BorderLayout component. The configuration properties for regions are included within the BorderLayout config, again, for convenience. The purpose of the config is to tell the BorderLayout component with the regions you want. I think the way it does it is pretty convenient, even if you have to specify empty configs to initialize a region.

barrygently
27 Nov 2006, 5:54 PM
Well I won't argue anymore about it (we'll have to just disagree), however can you add an error message then to indicate when the error condition arises? As I said, I had to trawl around the code trying to understand why it just failed (and it failed badly cause it then wouldn't layout anything but the center panel).