better cursor support for controls

[bartvde]

I want to start working on better cursor support for OL controls. Currently a lot of the cursors are hard-coded in javascript, without any way to override them.

I am doubting a bit between having control API properties, or doing this completely with css classes. I tend to be in favour of the API properties, since if we autogenerate css class names (ofcourse we could also work out something similar to displayClass, e.g. cursorClass), we will run into trouble with e.g. the ZoomBox Control, for which people will want a different cursor if out=true. With API Properties they will be able to do that.

When the tool is activated, the cursor on the map should be cursor X. But also in some cases people want the cursor to change on mouse down to cursor Y, for instance the pan hand in Google maps changes to a move cursor on mouse down. Also APIProperties will make it easier since we don't need a removeClass and addClass method for adding/removing css classes to the map div.

Initial proposal for APIProperties: -cursor -mousedownCursor

which would be set on the map div (this.map.div.style.cursor).

Thoughts welcome before I start my implementation. Especially with the decision to use css classes or to use API Properties for the cursor. TIA.

[bartvde]

Okay, work in progress in my sandbox called cursor:

 http://dev.openlayers.org/sandbox/bartvde/cursor/examples/cursor.html

[bartvde]

preliminary patch to give an idea about the approach

[bartvde]

There is a conflict between this approach and the OpenLayers.Control.Button items of NavigationHistory. Everytime an activate is called on those buttons, which sets the cursor back to default.

[tschaub]

I think I mentioned this approach elsewhere (can't remember if there was discussion on it), but I would suggest adding and removing a class name from the map div instead of setting the cursor property directly. My felling is that if we are setting style properties directly in the code, we're going to be limiting what can be accomplished.

If the map div is given an additional class name temporarily, we can have style declarations that determine the cursor property. Then when this class name is removed, whatever custom cursor the application designer might have set in the first place is restored.

With this approach (as far as I can tell from the patch), if someone sets a cursor in their own stylesheet, it is lost.

For methods to add and remove class names, see ( http://trac.openlayers.org/browser/sandbox/topp/almanac/lib/OpenLayers/BaseTypes/Element.js#L126 this). I'll make a patch for that.

[euzuro]

Replying to tschaub:

I think I mentioned this approach elsewhere (can't remember if there was discussion on it), but I would suggest adding and removing a class name from the map div instead of setting the cursor property directly. My felling is that if we are setting style properties directly in the code, we're going to be limiting what can be accomplished. If the map div is given an additional class name temporarily, we can have style declarations that determine the cursor property. Then when this class name is removed, whatever custom cursor the application designer might have set in the first place is restored. With this approach (as far as I can tell from the patch), if someone sets a cursor in their own stylesheet, it is lost. For methods to add and remove class names, see ( http://trac.openlayers.org/browser/sandbox/topp/almanac/lib/OpenLayers/BaseTypes/Element.js#L126 this). I'll make a patch for that.

.... i believe you mentioned it here: #688

[tschaub]

Replying to euzuro:

Replying to tschaub:

I think I mentioned this approach elsewhere (can't remember if there was discussion on it), but I would suggest adding and removing a class name from the map div instead of setting the cursor property directly. My felling is that if we are setting style properties directly in the code, we're going to be limiting what can be accomplished. If the map div is given an additional class name temporarily, we can have style declarations that determine the cursor property. Then when this class name is removed, whatever custom cursor the application designer might have set in the first place is restored. With this approach (as far as I can tell from the patch), if someone sets a cursor in their own stylesheet, it is lost. For methods to add and remove class names, see ( http://trac.openlayers.org/browser/sandbox/topp/almanac/lib/OpenLayers/BaseTypes/Element.js#L126 this). I'll make a patch for that.

.... i believe you mentioned it here: #688

Thanks for the reminder. Check #1607 for css class name management functions.

[crschmidt]

#1607 is going to go in this week as part of a sprint at MetaCarta. Once that is done, this ticket should be refactored to use that. Bart, if you update this (or someone else does) this can go into 2.7, but we don't plan to work specifically on this and it's not a blocker for 2.7.

[euzuro]

Lack of activity.

[bartvde]

Okay I've finally gotten the chance to rewrite this stuff, and am left with a few questions:

1) if a control has multiple handlers, like the dragFeature control, is it okay to pass the handlerOptions of the control to both handlers? Or is it necessary for the handlerOptions to have a hash per handler? 2) I haven't really taken an effort to mimic the current OpenLayers cursors used. Do people feel strongly about keeping cursors as they are, e.g. crosshair on box etc.? If so, I can make the effort the restore all the previous cursors. 3) I have adapted the DragFeature test, but would appreciate somebody more knowledgeable of the DragFeature control to have a look to determine if I did not break anything. All I could see is that cursor should change to move when over the feature.

The sandbox is still the same, bartvde/cursor, and the last patch is derived from there.

Please review.

TIA.

[bartvde]

New patch using addClass and removeClass

[avlee]

Using CSS to apply cursors is a good way.

[ksgeograf]

Very nice work! I have applied the revised patch to a 2.8 release, and it works very well.

Perhaps the controls (ZoomIn, ZoomOut and Drag) should have the cursorClass property assigned to a default value, rather than require this in the constructing code?

[bartvde]

Any chance one of the core developers can review this one? I am really eager to get this into 2.8, so I'll adapt/work more on the patch if necessary.

[tschaub]

Ok, the whole point of css is *not* to have to change your code to change the style. I kept meaning to work on this, but am not likely to find the time over the weekend. If someone really wants control over the cursors, have the code add and remove class names from appropriate elements. An element can have more than one class name. Pick a good default name. Then put the style declarations in the stylesheet. I'm sorry we have a difference in understanding about this - and perhaps I'm not being clear. Bottom line, you shouldn't have to write JavaScript to change the cursor.

[bartvde]

Hey Tim, working on this now. Will update the patch later today.

[tschaub]

While trac's preview isn't that satisfying, I'm hoping that the latest patch does satisfy.

[tschaub]

Replying to bartvde:

Hey Tim, working on this now. Will update the patch later today.

Bart, sorry I didn't see this comment until finishing the patch. Let me know if the changes I'm proposing work for you.

The idea is that we add and remove class names from the map div. We also provide sensible style declarations with class selectors in the default css. Users can customize style (cursor for example) by adding css declarations *only* - no need to modify any code.

I've added a declaration for dragging while the drag feature control is active to show how declarations in a more specific class selector can override those in a more general one.

e.g.

.olControlDragFeatureActive.olControlDragFeatureOver.olDragDown {
    cursor: -moz-grabbing;
}

(Note that though this is a proprietary cursor style, it does no harm where not supported.)

Tests pass in FF3.

[tschaub]

Also, I didn't reproduce your cursor.html example because I don't have the required cursor graphics, but if you attach them, I think we can make a simple example that shows how you can get custom graphics with css only.

[bartvde]

adding mouse cursors

[bartvde]

Hey Tim, this looks good to me. Tests pass in IE6 as well.

[bartvde]

Btw, this patch also has the same issue I was trying to tackle yesterday. In IE, when changing the cursor on pan mousedown (closed hand), it will only refresh the cursor after a mousemove unfortunately. In Google Maps this does not happen. IE6 will show the default cursor for the meantime, IE7 will show the previous cursor (open hand).

[bartvde]

More on this, IE6 only shows the default cursor if the previous cursor is url based, otherwise it will show the previous cursor just like IE7. If you tab out of the page to the location bar, the cursor changes. Some websites report that switching the display (none->block) of the map div should trigger the new cursor as well, but that's not working for me somehow.

[tschaub]

give control over cursors to users

[ahocevar]

Bart is right, but I don't find this issue annoying. Another thing though, which can probably easily be fixed with this patch, is that in IE7, when there is no tile of an Image layer and no vector feature underneath the cursor, the text cursor will be shown. This is no issue specific to this patch, it was the same before.

I will try to provide a new patch later today or tomorrow, but have an obligation now.

[ahocevar]

fixed default cursor in IE

[ahocevar]

Great work Bart and Tim. I added a default cursor to the map div class to fix the issue in IE7 that with no image tile or vector feature underneath the cursor there was a text cursor. I am not concerned about the move cursor being reset when moving the mouse only. Please commit.

[tschaub]

Thanks for the work Andreas! I'll check this out in a bit. I appreciate your review and the IE help.

[tschaub]

(In [9258]) Allowing users to better customize cursors for controls. Thanks to bartvde for the original patch and inspiration to get it done. Thanks ahocevar for the IE fix. r=ahocevar (closes #1484)