OverviewMap control #2769

adube · 2014-09-29T19:15:17Z

This replaces both the old #822 and #2444.

The following fixes were made so far:

(enhancement - fixed) use map postrender instead of view resolution, center and rotation property changes, as suggested by @tonio. The small box doesn't follow the animation of the map (i.e. while doing a pan/zoom), because the extent of the main map is always the same while animating, i.e. is equal to the end result. More about this.
(bug - fixed) listen to the map view change and act accordingly, as suggested by @elemoine and @tschaub .
(bug - fixed) defining the control with maximized: false no longer works
(task - fixed) follow the new button structure
(task - fixed) add a 'collapsible' option, as the attributions control, to allow the control to be drawn without a button

What's remaining to do, which includes all comments from #822 and #2444, in my personal order of priority :

(enhancement, requires One map per view #2735) use map corners instead of the extent as a way to get draw the box to allow it to animate smoothly while the map is animating.
(task) add tests
(task) support dragging the box to change the main map view (it would be really nice to have this)
(enhancement) support rotating the box instead of overview map (this could be left undone for now)

adube · 2014-09-30T15:20:28Z

The button structure of ol3 is now used for the overviewmap control. I "borrow" lots of css from the attribution control (i.e.: copy).

adube · 2014-09-30T15:58:59Z

ol3 currently does not have a method to get the map 4 corners, which is different from a view extent (if I understood correctly). This seem to be one of the topic of #2735. I'll wait for #2735 to be completed.

BTW, the overview map no longer assumes the map has a view. It doesn't do anything if it doesn't have one. Please, correct me if I'm wrong.

elemoine · 2014-09-30T16:20:01Z

Yep, having a getViewportCorners method would be useful. With #2735 it's not clear to me if that method should be on the map or on the view. This is one of the reasons I'm still not comfortable with #2735 – it blurs the boundary between the map and the view.

adube · 2014-09-30T18:13:19Z

I think the overviewmap control would be ready 'as-is'. What's remaining to do could be marked in separate issues. The control could use some tests, but I'd like to have it reviewed first. The tests could also be done after.

Thanks

elemoine · 2014-09-30T19:04:59Z

css/ol.css

+}
+.ol-overviewmap-box {
+  border: 2px dotted rgba(0,60,136,0.7);
+}


"No new line at end of file"

elemoine · 2014-09-30T19:54:17Z

Thanks @adube. I'm done reviewing for today. I'd like to look more closely at how the map and the overview map are synchronized and when the rendering of the overview map occurs.

adube · 2014-09-30T19:59:09Z

@elemoine thank you ! I should be able to complete all fixes you mentioned by tomorrow.

tschaub · 2014-10-01T05:05:58Z

Leaving #2735 out of this, let's discuss how the overview map should show the main map extent with rotation (and potentially with tilt).

Making it so the overview map is fully synchronized with the main map would be facilitated by animating the view (see https://github.com/tschaub/ol3/tree/animated-view).

tschaub · 2014-10-01T05:14:44Z

If you want to demonstrate two different uses of an overview map, I'd suggest two different examples. One that uses the defaults and one that uses custom options.

elemoine · 2014-10-01T05:58:48Z

The overview map could "just" draw a quadrilateral representing the main map viewport. Until we support tilt that quadrilateral will always be a rectangle. And also, I think I'd focus on the case where the overview map does not rotate with the main map. So the overview map would show a rotated rectangle when the main map is rotated. Is that making any sense?

elemoine · 2014-10-01T06:43:28Z

externs/olx.js

+ *     rotateBox: (boolean|undefined),
+ *     target: (Element|undefined),
+ *     tipLabel: (string|undefined)}}
+ * @api stable


You can remove stable here (but leave @api). It is useless and we don't have it for the other @typedefs. Stability is marked on individual properties.

adube · 2014-10-01T12:36:13Z

Following and agreeing with @tschaub suggestion, we now have two examples:

one with the default behaviour and style of the overviewmap control
the other featuring advanced customization using options and CSS

adube · 2014-10-01T14:00:36Z

I'm done with fixing everything from @elemoine review. Only two points remain undone, but those are trivial IMHO.

I'd be in favor of merging this pull request "as-is" (with any additional trivial fixes if required). Discussions / new features such as showing a rotated rectangle when the main map is rotated could come in an other pull request, in an other separated discussion.

After 1 and a half year of waiting, I'm eager to see this merged 😄

Thank you very much for your help

elemoine · 2014-10-01T16:12:14Z

src/ol/control/overviewmapcontrol.js

+   * @type {number}
+   * @private
+   */
+  this.minRatio_ = ol.OVERVIEWMAP_MIN_RATIO;


Why storing the value min and max ratio values in the instance? Why not use the constants in the code?

Done. Now using constants directly.

elemoine · 2014-10-07T17:28:44Z

I'd be in favor of merging this. It seems to me that the overview map control still needs some work, but I'd secure this work by merging it into master, and improve on it with new PRs.

And before merging I'd change all the @api stable annotations to @api (experimental), to give us the capacity to change the API while working on improvements in the future. I'd also squash the commits into, say, 3 commits: one for the control itself, and one for each example.

What do others think?

pagameba · 2014-10-07T19:15:11Z

I agree with the rationale for merging (I have not reviewed but have been following the thread).

ahocevar · 2014-10-08T08:51:10Z

+1 on merging without any @api stable annotations. @adube, can you please change them to just @api? Thanks.

adube · 2014-10-09T12:13:00Z

@ahocevar sure, I'm on it.

adube · 2014-10-09T13:26:39Z

There you go. Travis build fails, but I don't think it's caused by the content of this PR.

Thanks to all of you who helped me working on this control.

elemoine · 2014-10-09T13:30:45Z

@fredj any idea on the Travis build failure?

fredj · 2014-10-09T13:42:18Z

The examples must include jquery with:

<script src="../resources/jquery.min.js" type="text/javascript"></script>

instead of:

<script src="jquery.min.js" type="text/javascript"></script>

elemoine · 2014-10-09T13:50:08Z

Yep, I knew this was related to jquery moved to the resources dir :-) Thanks @fredj. @adube can you please fix your examples? Thanks.

adube · 2014-10-09T14:11:18Z

Done

elemoine · 2014-10-09T17:33:37Z

Thanks.

OverviewMap control

adube mentioned this pull request Sep 29, 2014

OverviewMap control #2444

Closed

elemoine reviewed Sep 30, 2014
View reviewed changes

elemoine reviewed Oct 1, 2014
View reviewed changes

adube force-pushed the overviewmap branch 3 times, most recently from 0cd615f to 1e7fbcf Compare October 9, 2014 13:21

adube added 3 commits October 9, 2014 10:10

Add OverviewMap control

ddc6138

Add overviewmap example, simple use case

f17bb1a

Add overviewmap-custom example, complex use case

efbeba3

adube force-pushed the overviewmap branch from d0c8c8b to efbeba3 Compare October 9, 2014 14:11

elemoine pushed a commit that referenced this pull request Oct 9, 2014

Merge pull request #2769 from adube/overviewmap

4c63609

OverviewMap control

elemoine merged commit 4c63609 into openlayers:master Oct 9, 2014

adube deleted the overviewmap branch October 9, 2014 18:25

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

OverviewMap control #2769

OverviewMap control #2769

adube commented Sep 29, 2014

adube commented Sep 30, 2014

adube commented Sep 30, 2014

elemoine commented Sep 30, 2014

adube commented Sep 30, 2014

elemoine Sep 30, 2014

adube Sep 30, 2014

elemoine commented Sep 30, 2014

adube commented Sep 30, 2014

tschaub commented Oct 1, 2014

tschaub commented Oct 1, 2014

elemoine commented Oct 1, 2014

elemoine Oct 1, 2014

adube Oct 1, 2014

adube commented Oct 1, 2014

adube commented Oct 1, 2014

elemoine Oct 1, 2014

adube Oct 1, 2014

elemoine commented Oct 7, 2014

pagameba commented Oct 7, 2014

ahocevar commented Oct 8, 2014

adube commented Oct 9, 2014

adube commented Oct 9, 2014

elemoine commented Oct 9, 2014

fredj commented Oct 9, 2014

elemoine commented Oct 9, 2014

adube commented Oct 9, 2014

elemoine commented Oct 9, 2014

OverviewMap control #2769

OverviewMap control #2769

Conversation

adube commented Sep 29, 2014

adube commented Sep 30, 2014

adube commented Sep 30, 2014

elemoine commented Sep 30, 2014

adube commented Sep 30, 2014

elemoine Sep 30, 2014

Choose a reason for hiding this comment

adube Sep 30, 2014

Choose a reason for hiding this comment

elemoine commented Sep 30, 2014

adube commented Sep 30, 2014

tschaub commented Oct 1, 2014

tschaub commented Oct 1, 2014

elemoine commented Oct 1, 2014

elemoine Oct 1, 2014

Choose a reason for hiding this comment

adube Oct 1, 2014

Choose a reason for hiding this comment

adube commented Oct 1, 2014

adube commented Oct 1, 2014

elemoine Oct 1, 2014

Choose a reason for hiding this comment

adube Oct 1, 2014

Choose a reason for hiding this comment

elemoine commented Oct 7, 2014

pagameba commented Oct 7, 2014

ahocevar commented Oct 8, 2014

adube commented Oct 9, 2014

adube commented Oct 9, 2014

elemoine commented Oct 9, 2014

fredj commented Oct 9, 2014

elemoine commented Oct 9, 2014

adube commented Oct 9, 2014

elemoine commented Oct 9, 2014