Opened 7 years ago

Closed 7 years ago

Last modified 7 years ago

#16275 closed defect (fixed)

dojox.mobile.Heading doesn't set up moveTo in all scenarios

Reported by: bedfordsean Owned by: Eric Durocher
Priority: undecided Milestone: 1.8.2
Component: DojoX Mobile Version: 1.8.1
Keywords: Cc:
Blocked By: Blocking:

Description (last modified by bill)

When using the dojox.mobile.Heading, I noticed some strange behaviour whilst attempting to programatically change the moveTo location.

I have instantiated my heading in HTML:

<h1 data-dojo-type="dojox/mobile/Heading"
	data-dojo-attach-point="_viewHeader"
	data-dojo-props="label:'Hello',
	back:'Back',
        fixed:'top'"></h1>

and in my postCreate() I do:

this._viewHeader.backButton.set("moveTo", this.movedFrom);

This _appears_ to set the right moveTo parameter for the heading. However, when I transition to the view and try to use the back button, nothing happens.

Interestingly, if I do the following in my HTML definition:

<h1 data-dojo-type="dojox/mobile/Heading"
	data-dojo-attach-point="_viewHeader"
	data-dojo-props="label:'Hello',
	back:'Back',
        '''moveTo:'SomeNonExistentView','''
        fixed:'top'"></h1>

Then this works.

I have debugged the code around Heading.js, and feel that it could benefit from a _setMoveToAttr() function so that this can be done directly:

this._viewHeader.set("moveTo", this.movedFrom);

However, this still doesn't change the fact that nothing happens when the back button is tapped if no moveTo parameter was set in the first place.

I am guessing that an onClick event is connected to the back button upon the instantiation of the header only if the moveTo parameter is defined upon instantiation.

This should probably be changed so that if a moveTo is set at any point, the correct event is connected to the button (this could be put into the outline for a _setMoveToAttr() that I have attached below).

---------------------------------------------------------
dojox/mobile/Heading.js 
---------------------------------------------------------
		_setMoveToAttr: function(/*String*/moveTo){
			// tags:
			//		private
			this._set("moveTo", moveTo);
			if(this.backButton){
				this.backButton.set("moveTo", this.moveTo);
			}
			this.resize();
		},

---------------------------------------------------------

Attachments (1)

patch16275.patch (1.9 KB) - added by Adrian Vasiliu 7 years ago.
Heading now supports setting the moveTo after construction. Added test case in DOH test. - Adrian Vasiliu, IBM, CCLA

Download all attachments as: .zip

Change History (9)

comment:1 Changed 7 years ago by bill

Component: GeneralDojoX Mobile
Description: modified (diff)
Owner: set to Eric Durocher

comment:2 Changed 7 years ago by Adrian Vasiliu

Yes, you correctly guessed what's going on. There are a number of properties that are currently meant to be set at initialization time and are not meant to be changed afterwards. Now, to allow all such properties to be set at any time requires adding a bunch of code, while dojox.mobile tries to stay as light as possible... So this is a matter of tradeoff. We might fix a limited number of cases (say, the most frequent ones, which probably include Heading's moveTo case) while relying on improved documentation for the others.

For now, the simplest way to get it working is something around these lines:

var myHeading = registry.byId("heading1"); // registry is "dijit/registry"
myHeading.set("moveTo", "newTarget");
if (myHeading.backButton) {
  myHeading.backButton.set("moveTo", "newTarget");
}

(By the way, I do not think the "this.resize();" that you've put at the end of the custom setter is necessary. It is necessary in the setter of the "back" property, but not here.)

Last edited 7 years ago by Adrian Vasiliu (previous) (diff)

Changed 7 years ago by Adrian Vasiliu

Attachment: patch16275.patch added

Heading now supports setting the moveTo after construction. Added test case in DOH test. - Adrian Vasiliu, IBM, CCLA

comment:3 Changed 7 years ago by bedfordsean

I can confirm that the patch above works as expected.

Would also recommend an update to the dojox/mobile/Heading documentation to state that moveTo is a mandatory attribute (or, at least it is for correct behaviour!).

comment:4 Changed 7 years ago by Adrian Vasiliu

The moveTo isn't mandatory, for instance the Heading can be used without any back button (the property "back" not being set), or it can be used with "back" and "href" specified instead of "moveTo". I don't mean the doc couldn't be made clearer, but I'm not sure what you mean by asking for the doc to state that moveTo is mandatory.

comment:5 Changed 7 years ago by bill

#16315 is a duplicate of this ticket.

comment:6 Changed 7 years ago by cjolif

Milestone: tbd1.8.2

comment:7 Changed 7 years ago by cjolif

Resolution: fixed
Status: newclosed

In [29943]:

fixes #16275. Heading now supports setting the moveTo after construction. Added test case in DOH test. Adrian Vasiliu (IBM, CCLA).

comment:8 Changed 7 years ago by cjolif

In [29944]:

refs #16275. Backport to 1.8. Heading now supports setting the moveTo after construction. Added test case in DOH test. Adrian Vasiliu (IBM, CCLA).

Note: See TracTickets for help on using tickets.