navigation history control

This control is intentionally generic so that it could be used to create a history control superclass. The only properties/methods specific to the navigation history are on, restore, and initStack. All others could go in OpenLayers.Control.History. Subclasses of the history control can be used to do things like page through previous/next edits on a vector layer (once we start triggering appropriate events on the vector layer).

I'll write tests after the design is reviewed.

Example here: http://dev.openlayers.org/sandbox/tschaub/history/examples/navigation-history.html

Replying to tschaub:

This control is intentionally generic so that it could be used to create a history control superclass. The only properties/methods specific to the navigation history are on, restore, and initStack. All others could go in OpenLayers.Control.History. Subclasses of the history control can be used to do things like page through previous/next edits on a vector layer (once we start triggering appropriate events on the vector layer). I'll write tests after the design is reviewed.

Very very nice.

One question: everything seems to be based on map events, meaning that things that don't generate map events cannot be saved/restored. Do you confirm my understanding? (I'm not saying this is a bad thing).

Pretending all of that code with the exception of on, restore, and initStack was in OpenLayers.Control.History, a class called OpenLayers.Control.FeatureHistory (or something similar) might have an initialize method that looked like this:

initialize: function(layer, options) {
    this.events = layer.events;
    OpenLayers.Control.History.prototype.apply(this, options);
},

and then it's on property might look like:

on: {
    "featuremodify": function(evt, feature) {
        return {
            feature: feature, geometry: feature.geometry.clone()
        };
    }
},

Then of course the modify feature control would have to trigger "featuremodify" on the layer events - and send the modified feature. And that control's restore method would deal with restoring a feature's previous/next geometry. Or something like that.

Anyway, the control defaults to map events, but the events property events can be set to any other events instance (something I already wish we had in handlers).

Tim,

would it be an option to have a control property direction with values forward and backward? So that things can just be handled the same way as other controls (API-wise)? One would just call trigger like with other controls of type OpenLayers.Control.TYPE_BUTTON.

So e.g.:

var nav_fwd = new OpenLayers.Control.NavigationHistory({direction: 'forward'}); 
var nav_back = new OpenLayers.Control.NavigationHistory({direction: 'backward'}); 

Tim, I noticed that if the control is added after map.zoomToMaxExtent has already been called that things don't work. I get a javascript error in that case:

this.on has no properties
(no name)()OpenLayers.js (line 6097)
initStack()OpenLayers.js (line 6148)
activate()OpenLayers.js (line 6134)
draw()OpenLayers.js (line 6017)
addControlToMap(Object displayClass=olControlNavigationHistory, undefined)OpenLayers.js (line 10767)
addControl(Object displayClass=olControlNavigationHistory, undefined)OpenLayers.js (line 10751)
initToolbar()mapviewer20.js (line 127)
initMap(div#center1)mapviewer20.js (line 60)
(no name)()mapviewer20 (line 109)
Observable()ext-all.js (line 12)
[Break on this error] this.on[type], this

So either this should be documented, or resolved.

Replying to bartvde:

Tim, I noticed that if the control is added after map.zoomToMaxExtent has already been called that things don't work. I get a javascript error in that case: {{{ this.on has no properties (no name)()OpenLayers.js (line 6097) initStack()OpenLayers.js (line 6148) activate()OpenLayers.js (line 6134) draw()OpenLayers.js (line 6017) addControlToMap(Object displayClass=olControlNavigationHistory, undefined)OpenLayers.js (line 10767) addControl(Object displayClass=olControlNavigationHistory, undefined)OpenLayers.js (line 10751) initToolbar()mapviewer20.js (line 127) initMap(div#center1)mapviewer20.js (line 60) (no name)()mapviewer20 (line 109) Observable()ext-all.js (line 12) [Break on this error] this.on[type], this }}} So either this should be documented, or resolved.

Apologies for that. I've fixed it in my sandbox. New patch to come.

Replying to bartvde:

Tim, would it be an option to have a control property direction with values forward and backward? So that things can just be handled the same way as other controls (API-wise)? One would just call trigger like with other controls of type OpenLayers.Control.TYPE_BUTTON. So e.g.: {{{ var nav_fwd = new OpenLayers.Control.NavigationHistory({direction: 'forward'}); var nav_back = new OpenLayers.Control.NavigationHistory({direction: 'backward'}); }}}

One issue here is that you need a single history stack for next/previous to work as you'd expect. With the above configuration, you'll have two stacks that don't know about each other. I had imagined these history controls to be meta-controls of a sort. Meaning that you might have two visual representations (buttons) of the same control. If you want those visual representations to be OL controls, then you can make simple button controls like so:

                nav = new OpenLayers.Control.NavigationHistory();
                map.addControl(nav);
                
                prev = new OpenLayers.Control({
                    type: OpenLayers.Control.BUTTON,
                    trigger: function() {nav.previous();},
                    active: true
                });
                map.addControl(prev);
                
                next = new OpenLayers.Control({
                    type: OpenLayers.Control.BUTTON,
                    trigger: function() {nav.next();},
                    active: true
                });
                map.addControl(next);

I've also updated my sandbox code to make the NavigationHistory control a button type control with a trigger method. The behavior of the trigger method is controlled by the "backwards" property (boolean). By default (backwards true), trigger calls previous. If backwards is false, trigger calls next.

I've also given the trigger method an options argument. If options.opposite is true, then the behavior of trigger is reversed. The idea would be that you could have a single visual representation (button) with different behavior based on some condition (like shift or ctrl key modifiers). For this to work all in OL (in a panel), we'd need the panel.onClick method to pass along additional arguments to panel.activateControl (like the click event itself). Then activateControl would call trigger (or activate or deactivate) with an options arg like {evt: evt} - leaving the controls to decide what to do with that event (like check for key modifiers and modify their own behavior).

All that is a bit weird. I can't really imagine a history control with only a single button. I think you need two buttons (one that knows about control.previous and one that knows about control.next). Whether or not these buttons are OL controls is up to the designer.

Make sense?

The one remaining issue is what to do with an empty stack (in either direction). It makes sense that you'd want to disable the next button when there are no more items in the next stack (or same with previous).

Currently, next and previous return a representation of the state they are restoring. If this is null, the stack is empty - so a button could disable itself if the return is null. However, this means there will be one click of the button that does nothing before the button is disabled.

The real solution is for our controls to have events instances (or something similar). Controls could register listeners for events like "previouschanged" or "nextchanged" where the listener would get the state representation and maybe the length of the remaining stack.

Until then we're stuck with overriding methods like onPrevious or onNext (meaning that only one thing gets to "listen" to one control). Any other ideas would be appreciated.

Hi Tim, random thoughts ...

I'm not sure if this is good programming practice or not, but the way I implemented a similar control for Fusion was to use a class variable for the state so it was shared between all instances of the widget, and each instance then had a direction (forward or backward). I used a single "historychanged" event that the widget registered for on itself so that each instance could decide whether to be enabled or not depending on the current index into the history stack. This class variable approach could be used for the current two stack implementation.

Or the history could be stored as part of the Map and the API for next/previous could exist there as well. Not sure that I like this idea, but I thought I'd throw it out there anyway.

I do prefer two buttons over one. There also may be some advantage to tying into the browser's forward/back buttons and using a framework like http://code.google.com/p/reallysimplehistory/.

Another thought that occurred to me is that it may be desirable to have generic do/undo that includes anything that could be pushed onto the stack. This is much more ambitious and would require pushing an object that is capable of undoing / redoing something like an extent change.

Ok, instead of my above suggestion of having the application designer construct two button controls that work with the navigation history control, this code has now been incorporated into the constructor for the control itself. So, you now do something like:

var nav = new OpenLayers.Control.NavigationHistory();
map.addControl(nav);
nav.previous.trigger(); // one step back in history
nav.next.trigger(); // one step forward in history

Those properties (previous and next) refer to full fledged button controls. They get activated when the appropriate stacks are available and deactivated when not.

The one missing piece is providing a way for the application designer to know when those dependent controls are activated/deactivated. I've given the sub-controls onActivate and onDeactivate methods. This is sub-optimal, but until we give controls an events instance, it follows the pattern elsewhere.

Currently, controls don't do any form of notification when they become active/deactive. So, if you have a control that is part of a panel and you call control.activate(), the panel won't know to redraw itself. These sub-controls suffer from the same problem - which is really a problem at the panel level.

So, I think the long-term solution is control.events. Until then, onActivate and onDeactivate are the most sensible solutions.

And, Paul, I think your comments are good ones. For now, I like the idea of separating different forms of history handling. Eventually, someone could tie in the browser buttons, but we'll always want to be able to keep them separate as well. As mentioned above, I think this code makes a good generic History superclass (with the exception of on, restore, and initStack). Once we get a couple implementations, we'll have a better idea what the API should be.

Hey Tim, I think you used the wrong constant value here: it should be OpenLayers.Control.TYPE_BUTTON instead of OpenLayers.Control.BUTTON. This is quite funny though seeing your recent comment for ticket:1187 :-)

Tim, other than the constant typo mentioned above it worked fine for me. I was able to enable/disable the buttons in the MapFish toolbar.

Very good point Bart. I did mention that I am a terrible speller, didn't I :)

is the value of 'on' something that people will ever want to override on the history control? putting objects in the class prototype doesn't work in general, because they're class variables, so they affect all instances, I think. (Erik could probably be more accurate.)

Replying to crschmidt:

is the value of 'on' something that people will ever want to override on the history control? putting objects in the class prototype doesn't work in general, because they're class variables, so they affect all instances, I think. (Erik could probably be more accurate.)

I'll clarify my thinking here in 1 week.

See #659 for some images we may want to consider ganking as default styling for these controls.

Replying to crschmidt:

See #659 for some images we may want to consider ganking as default styling for these controls.

Thanks for pointing these out. The update patch inherits from the new button class (go buttons!) and uses the graphics attached to that old ticket (with mods from me).

It's still a bit weird to add this to a panel for two reasons: 1) panels don't know to redraw themselves when controls are activated/deactivated by means other than a click (addressed by adding events instances to controls), and 2) you have to add the parent control to the map (so its draw method gets called) even if you are adding the sub-controls to a panel

I could give the sub-controls a reference to their parent and have them call draw there if the parent doesn't already have a map, but that is a ticket for later. This thing is good as is - and gives flexibility in interface design.

Replying to crschmidt:

is the value of 'on' something that people will ever want to override on the history control? putting objects in the class prototype doesn't work in general, because they're class variables, so they affect all instances, I think. (Erik could probably be more accurate.)

Putting objects in a prototype is fine as long as we only want people to override those objects wholesale. (Since we extend with the options arg, you can't update a single property of an object on the prototype with the options arg.) That said, someone could do control.on.whatev and modify the on object on the prototype without knowing it.

My reason for putting it on the prototype was to make writing a generic History class a cinch, and then have subclasses only define that one property (and the initStack method). Anyway, I'll wait until the need arises. And I can always put it in ALL_CAPS to make sure everybody knows not to modify it (THOUGH_IM_NOT_A_FAN_OF_ALL_CAPS).

Anyway, I renamed "on" to "registry" to save room for an "on" method in the future. More on that later.

Ok, this one is ready to go. The example works as advertised in FF, IE6, IE7, Opera, and Safari. Tests pass in the same browsers.

The one issue revealed by this change is that IE sometimes chokes when we are removing controls from the map (when you navigate away from the example page in IE). This is addressed separately in #1316.

Will gladly commit with review.

I took a look at your patch yesterday night before I went to sleep. I didn't really take part to the discussion, so I can't really say "ok, go and commit". The code looks nice to me though, clear and easy to understand.

However, here is one tiny suggestion for your patch. You should probably put a "var" in front of lines 8 and 21 in tests/Control/test_NavigationHistory.html for the "control" variable.

My 2 cents.

Replying to pgiraud:

I took a look at your patch yesterday night before I went to sleep. I didn't really take part to the discussion, so I can't really say "ok, go and commit". The code looks nice to me though, clear and easy to understand. However, here is one tiny suggestion for your patch. You should probably put a "var" in front of lines 8 and 21 in tests/Control/test_NavigationHistory.html for the "control" variable. My 2 cents.

Thanks for taking a look. I'll correct the control scoping when I commit. Though this ticket has many comments, I think the discussion is effectively done. Pipe up if I'm wrong.

As mentioned in the comments, the parent history navigation control is not intended to be added to a control panel. The sub-controls (previous and next) can be added to a panel as buttons. If you do choose to have the parent control in a panel, it will be added as a toggle with the updated patch. No significant change, works, tests pass, etc. Still up for review.

control navigation history

and the image graphics

Great work, The patch looks good to me. It does not have any impact on existing code, and there was enough discussion to be sure that we cover all use cases to start with. Please go ahead and commit.

(In [6157]) Add navigation history control. This control creates two obligate controls: next and previous. Calling trigger on the next and previous controls steps through the navigation history. r=crschmidt,ahocevar,pgiraud (closes #1240)