OverviewMap overhaul

[tschaub]

The overview map control has a good bit of code that is now encapsulated in the drag handler and click handler. Easier to maintain there. The extent rectangle should also be replaced when it gets too small.

[tschaub]

$ svn diff lib | diffstat
 OverviewMap.js |  118 ++++++++++-----------------------------------------------
 1 file changed, 22 insertions(+), 96 deletions(-)

Tests pass, examples work. Please review.

[tschaub]

Updated patch.

$ svn diff lib | diffstat
 OverviewMap.js |  217 ++++++++++++++++-----------------------------------------
 1 file changed, 64 insertions(+), 153 deletions(-)

I'm removing the review state, as there are a couple issues, but I like this direction. The overview map now has a click handler and a drag handler instead of all that custom event code.

[tschaub]

Ok, I'm happy with this. It makes use of better code and is more solid behavior. This exposed an issue with the click handler (see #1246), so apply that patch before reviewing.

I think the map.dragging property is a useful addition. To retrieve that information by another method would be ugly - and this is a simple change.

Thanks for the review.

[tschaub]

One other thing I noticed here. The drag handler stops propagation on mousedown (from getting to listeners on the same element). This makes sense when you have two controls with conflicting behavior on mousedown - but it is not always necessary. On the overview map, for example, it would be nice if the click handler got those mousedown events (so that it could do its pixel tolerance bit). I think it makes sense to give the drag handler a stopDown property that defaults to true. I'll make a ticket, but just wanted to document this while the justification was fresh.

[tschaub]

The previous changes were in preparation for replacing the overview map rectangle when it gets too small (#472). In hope of streamlining the review process, I'm including those changes here.

[tschaub]

improve the overview map control

[tschaub]

Tests pass and works in IE/FF.

[bartvde]

Tim, works for me. Is it really the best approach though to use the minRectSize as the width and height of the point image? Should you not use width and height auto for this?

[pspencer]

Tim, couple of very minor comments on the patch:

• minRectSize is documented as having a default of 10 but the actual default value is 15
• the example CSS for the RectReplacement CSS class is different from the actual definition in the patch, not sure if this is important or not

I have read the patch and looked closely at how it is changing the code and tested it from Tim's sandbox. I don't see any reason not to commit this.

[tschaub]

Thanks for checking this out pspencer. I'll make sure to clean that up.

[tschaub]

Replying to bartvde:

Tim, works for me. Is it really the best approach though to use the minRectSize as the width and height of the point image? Should you not use width and height auto for this?

Hey Bart. Let me know if I'm missing your point, but I think I get it. The minRectSize property is used to set the width/height of the extentRectangle when minRectSize is reached. The extentRectangle also gets a new display class name. The default CSS declaration gives the extentRectangle a background graphic. So I never touch the size of the image (just the size of the element that uses the graphic as a background). This is necessary to be able to position the extentRectangle (with top and left).

As an alternative, we could honor the height and width declared in the CSS - but then you'd have to type those numbers in twice (width and height) - seemed easier to configure the control with a single property. In addition, there's not a handy method to get a CSS rule for an element that is not currently rendered (as would be the case with the replacement rectangle - we need to know the threshold before deciding whether to switch styles).

[bartvde]

Hey Tim, sorry for the confusion then, I was looking with Firebug, and didn't notice it was the div around the image which set the size :-) . What I was getting is that my image (20x20) was squeezed into the 5x5 square.

[tschaub]

(In [5674]) Gutting the OverviewMap control to give it an update. Now uses a click handler and a drag handler instead of its own event handling code. In addition, the control now has a minRecSize property. When the extent rectangle is smaller than the specified size, its style is switched. By default, we provide a rectangle replacement graphic. This can be customized with CSS. Finally, I'm tucking in one non-API property. The dragging arg sent to map.setCenter is now stored at map.dragging. This gives easy reference to dragging state where a reference to the navigation control and its drag handler is not convenient. r=pspencer (closes #1244)

[tschaub]

Replying to bartvde:

Hey Tim, sorry for the confusion then, I was looking with Firebug, and didn't notice it was the div around the image which set the size :-) . What I was getting is that my image (20x20) was squeezed into the 5x5 square.

Yeah, so one limitation is that you can't display an image that is larger (in either dimension) than minRectSize - as background images don't bleed outside the bounds of the element they are associated with. But the control doesn't do any stretching/squeezing to resize images.

Please open another ticket if this doesn't work as expect - or fit your use case.