OpenLayers OpenLayers

Ticket #1244 (closed feature: fixed)

Opened 1 year ago

Last modified 1 year ago

OverviewMap overhaul

Reported by: tschaub Assigned to: tschaub
Priority: minor Milestone: 2.6 Release
Component: Control.OverviewMap Version: 2.5
Keywords: Cc:
State: Complete

Description (Last modified by 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.

Attachments

ovmap.patch (18.3 kB) - added by tschaub on 01/04/08 17:23:05.
improve the overview map control

Change History

01/03/08 15:22:25 changed by tschaub

  • status changed from new to assigned.
  • state set to Review.
$ svn diff lib | diffstat
 OverviewMap.js |  118 ++++++++++-----------------------------------------------
 1 file changed, 22 insertions(+), 96 deletions(-)

Tests pass, examples work. Please review.

01/03/08 23:59:45 changed by tschaub

  • state deleted.
  • description changed.
  • summary changed from give the OverviewMap control a drag handler to improve event handling for OverviewMap control.

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.

01/04/08 03:14:50 changed by tschaub

  • state set to Review.

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.

01/04/08 13:25:07 changed by 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.

01/04/08 15:56:53 changed by tschaub

  • description changed.
  • summary changed from improve event handling for OverviewMap control to OverviewMap overhaul.

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.

01/04/08 17:23:05 changed by tschaub

  • attachment ovmap.patch added.

improve the overview map control

01/04/08 17:25:50 changed by tschaub

Tests pass and works in IE/FF.

(follow-up: ↓ 10 ) 01/07/08 10:15:23 changed by 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?

01/07/08 13:19:41 changed by pspencer

  • state changed from Review to Commit.

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.

01/07/08 13:21:21 changed by tschaub

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

(in reply to: ↑ 7 ) 01/07/08 15:10:16 changed by 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).

(follow-up: ↓ 13 ) 01/07/08 15:15:29 changed by 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.

01/07/08 15:54:28 changed by tschaub

  • status changed from assigned to closed.
  • state changed from Commit to Complete.
  • resolution set to fixed.

(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)

(in reply to: ↑ 11 ) 01/07/08 19:20:09 changed by 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.