Opened 9 years ago

Closed 9 years ago

#10786 closed enhancement (wontfix)

enhance dijit._Templated to call this.inherited for buildRendering

Reported by: ben hockey Owned by:
Priority: high Milestone: tbd
Component: Dijit Version: 1.4.0
Keywords: Cc:
Blocked By: Blocking:

Description

i want to be able to mixin dijit._Templated into dijit.layout.BorderContainer? so i can do the following

dojo.declare('someWidget', [dijit.layout.BorderContainer, dijit._Templated], {
   gutters: false,
   design: headline,
   // etc.
})

template

<div>
   <div dojoType='something' region='top'></div>
   <div dojoType='anotherThing' region='center'></div>
</div>

for this to work, dijit._Templated needs to call this.inherited. the only thing preventing this is that dijit._Widget overrides this.domNode in buildRendering

this.domNode = this.srcNodeRef || dojo.create('div');

if dijit._Widget would check if the domNode already existed then dijit._Templated could safely call this.inherited at the end of it's buildRendering.

i think the following change to dijit._Widget would be backwards compatible and would allow this change to be made to dijit._Templated

	if (!this.domNode){
		this.domNode = this.srcNodeRef || dojo.create('div');
	}

Change History (6)

comment:1 Changed 9 years ago by bill

It seems odd for _Templated to call this.inherited() when it has already built the rendering. Why does it need to?

comment:2 Changed 9 years ago by ben hockey

in my specific use case shown in the description i want to mixin dijit._Templated to an already existing widget - dijit.layout.BorderContainer?. i want to extend BorderContainer? and have it's children defined in a template.

i have 2 options

  • i need to put _Templated between _Widget and _Container so i can't use _LayoutWidget so i start over wth dijit._Widget, mixin dijit._Templated, then mixin _Container and _Contained, add the code added by _LayoutWidget and finally add the code added by BorderContainer?
  • i can try to mixin dijit._Templated to the existing BorderContainer?.

in the latter case, i would prefer dijit._Templated to call this.inherited so that the buildRendering of dijit._Container (inherited by BorderContainer?) is called.

i know that 'i want...' is not a very convincing argument so the more general argument is that dijit._Templated is a mixin which implements buildRendering and it might not always be the only/last class to build the rendering.

it turns out that in my specific case above, if i put a dojoAttachPoint to make the domNode also be the containerNode then i can effectively do what _Container is doing and it wouldn't matter if _Templated is changed to call inherited or not.

if you think it's a weak argument then go ahead and close the ticket. it won't hurt my feelings :)

comment:3 in reply to:  2 Changed 9 years ago by ben hockey

Replying to neonstalwart:

...i want to mixin dijit._Templated to an already existing widget - dijit.layout.BorderContainer?.

this should read as:

...i want to mixin dijit._Templated to an already existing class - dijit.layout.BorderContainer?.

just wanted to clarify that i'm trying to extend the class - not just a single instance.

comment:4 Changed 9 years ago by bill

Oh OK, so the issue is _Container.buildRendering(), which sets this.container=this.domNode.

Yes, I see the inheritance-order problem you are talking about. I'll think about that.

comment:5 Changed 9 years ago by ben hockey

i've been thinking about this some more... unless you're really convinced that _Templated.buildRendering should call this.inherited, then i'm going to side with not doing it now. once it's done then there's no turning back when it comes to backwards compatibility in the future. doing this shouldn't break any backwards compatibility now but it means more cases to maintain in the future. all it takes is one small edge case that becomes possible by doing this and then that edge case needs to be supported through to 2.0.

for my specific case i can mark a node as the containerNode in the template and it won't matter that _Container.buildRendering isn't called. this is really no overhead since this is usually what you would do in the template anyway.

so, for the sake of not opening the codebase up to some unforeseen edge case, i favor not doing this now. if you feel that you have a reason to do this that outweighs the potential cost of maintaining it through to 2.0 then go ahead but as far as i'm concerned it would be ok to just close this ticket.

comment:6 Changed 9 years ago by bill

Resolution: wontfix
Status: newclosed

I agree, I don't think _Templated.buildRendering() should call _Widget.buildRendering().

I was hoping for some way that _Templated could get spliced into the inheritance chain, such that it would go BorderContainer --> _Container --> _Templated --> _Widget, but that doesn't seem trivial either.

So I'll close this or now, as you suggested.

Note: See TracTickets for help on using tickets.