Ticket #2529440 (closed defect)
Reporter Eduardo Lundgren |
Opened: 10/13/10 Last modified: 07/27/11 Status: closed Type: defect Resolution: fixed |
Owner Tilo Mitra |
Target Release: 3.4.0 Priority: P3 (normal) |
|---|---|---|---|
| Summary: | WidgetStdMod setStdModContent method is only valid for use after render | ||
| Description: | WidgetStdMod setStdModContent method is only valid for use after render, worth changing this to be available at any time of the Life-cycle? |
||
| Type: | defect | Observed in Version: | development master |
| Component: | Widget | Severity: | S3 (normal) |
| Assigned To: | Tilo Mitra | Target Release: | 3.4.0 |
| Location: | Priority: | P3 (normal) | |
| Tags: | Relates To: | ||
| Browsers: | N/A | ||
| URL: | http://jsfiddle.net/apipkin/Jjb9c/ | ||
| Test Information: | |||
Change History
|
Posted: 10/13/10
|
|
Posted: 12/2/10
|
|
Posted: 01/20/11
|
|
Posted: 01/20/11
I agree it does get a bit verbose using the full method/constants, and those shortcuts would be welcome. Where I've run into issues often is in creating extensions that also mix in WidgetParent, I want to set the _defaultChildContainer to one of the StdMod nodes, but as it stands I have to catch the rendering of children and halt it until the syncUI phase when those nodes exist. |
|
Posted: 01/20/11
Luke, really like that idea, not totally sold on the naming: Greg, I ran into the same issue and also solved it by doing work in syncUI, hopefully this will be fixed in the next release... |
|
Posted: 01/24/11
|
|
Posted: 06/9/11
|
|
Posted: 06/27/11
|
|
Posted: 07/25/11
I checked in some changes to WidgetStdMod that fixes this issue. The problem was being caused by the fact that at renderUI(), the attribute change listeners were not set up. I moved the setup of those listeners to the _renderUIStdMod() method. The rest of the listeners are still being setup in _bindUIStdMod(). I also added the convenience methods head(), body(), foot() that return a Y.Node instance of the appropriate sections. I added the test case provided by Eduardo to the src/widget-stdmod/tests/ directory. Give it a try and let me know what you think. I'll change the status to checkedin for now. |
|
Posted: 07/25/11
|
|
Posted: 07/26/11
I'm not a big fan of head(), foot(), body() - I'd prefer getHeader(), getFooter(), getBody() to make what they do more obvious. In general I'm of the opinion that methods should be verbs, not nouns. They're doing something, not naming something. |
|
Posted: 07/27/11
I took out head(), body(), foot() but didn't put in getHeader(), getFooter(), getBody() just because I think it may confuse newcomers to the library with something like get('header') - probably just a naming issue. That's something that I can add in for the next PR. I'm closing this ticket because the issue mentioned here is fixed to the best of my knowledge. I'll reopen a new ticket for the std-mod convenience method and link it here. |
Greg Hinch
I brought this up in conversation with Satyen previously, but didn't ticket it. A suggestion I had to simplify WidgetStdMod is to add methods head(), body(), and foot() to the host class. These methods would return the Node corresponding to the yui3-widget-hd, -bd, and -ft respectively. If the Nodes have not yet been created, they would be at that point. This would allow
var overlay = new Y.Overlay();
overlay.head().setContent("Title text");
overlay.body().empty();
etc
It is significantly more terse than overlay.getStdModNode(Y.WidgetStdMod.HEAD) and more intuitive than overlay.setStdModContent(Y.WidgetStdMod.BODY, "New content");
The returned nodes need only be generated on request. They could be attached to the container element in either renderUI or syncUI.