defaultZoom is really maxZoom?

21 views
Skip to first unread message

Paul Winkler

unread,
May 31, 2011, 2:51:59 PM5/31/11
to olwidget
I was just wondering why zoomToDataExtent didn't seem to actually do
anything, then I noticed this around line 316:

this.zoomTo(Math.min(this.getZoom(), this.opts.defaultZoom));

So zoomToDataExtent will actually never zoom more than defaultZoom.

Is defaultZoom misnamed, or is that line just wrong, or what? I was
expecting that defaultZoom would only get used if there was no
dataExtent to zoom to, eg. no features... so I was passing a
defaultZoom that gives me the whole area my map covers. So that's why
zoomToDataExtent doesn't zoom for me.

Charlie DeTar

unread,
Jun 1, 2011, 10:21:51 AM6/1/11
to olwi...@googlegroups.com

The issue is that if the data is a single point, the data extent is
infinitely small, and OpenLayers will zoom too far in for many base
layers. So a sensible default of "zoomToDataExtent" breaks down if the
geometry might be a single point.

I agree that the current solution probably isn't ideal, as it doesn't
give sensible defaults for the most common cases of "show me some zoom
level when there is no geometry" and "show me the geometry if there is one".

Currently, the following options control zooming:

zoomToDataExtent: (default true)
defaultZoom: (default 4)
mapOptions: {
numZoomLevels: (layer dependent default)
}

I'm hesitant to just throw more options in, as there are already so
many, but it seems like in this case it would make sense to add a
further option:

minInitialZoom

which would replace defaultZoom on line 316. This is different from the
idea of a "minZoom" that to me expresses a limit on zooming.

Does this seem like a reasonable solution?

It looks like philipn has been trying a similar solution here:

https://github.com/philipn/olwidget/commit/5236d058e01c16dc35a4e416aac19dcaca4c7b0f

though I find the name zoomToDataExtentMin a little cumbersome.

-charlie


Paul Winkler

unread,
Jun 1, 2011, 10:59:27 PM6/1/11
to olwidget
On Jun 1, 10:21 am, Charlie DeTar <cha...@gmail.com> wrote:
> On 05/31/2011 02:51 PM, Paul Winkler wrote:
>
> > I was just wondering why zoomToDataExtent didn't seem to actually do
> > anything, then I noticed this around line 316:
>
> > this.zoomTo(Math.min(this.getZoom(), this.opts.defaultZoom));
>
> > So zoomToDataExtent will actually never zoom more than defaultZoom.
>
> > Is defaultZoom misnamed, or is that line just wrong, or what?  I was
> > expecting that defaultZoom would only get used if there was no
> > dataExtent to zoom to, eg. no features... so I was passing a
> > defaultZoom that gives me the whole area my map covers.  So that's why
> > zoomToDataExtent doesn't zoom for me.
>
> The issue is that if the data is a single point, the data extent is
> infinitely small, and OpenLayers will zoom too far in for many base
> layers.  So a sensible default of "zoomToDataExtent" breaks down if the
> geometry might be a single point.

Huh, I assumed it was zooming to the max zoom, which presumably you
have
chosen because it works with your base layers. It's not?

> I agree that the current solution probably isn't ideal, as it doesn't
> give sensible defaults for the most common cases of "show me some zoom
> level when there is no geometry" and "show me the geometry if there is one".
>
> Currently, the following options control zooming:
>
>   zoomToDataExtent: (default true)
>   defaultZoom: (default 4)
>   mapOptions: {
>     numZoomLevels: (layer dependent default)
>   }
>
> I'm hesitant to just throw more options in, as there are already so
> many, but it seems like in this case it would make sense to add a
> further option:
>
>   minInitialZoom
>
> which would replace defaultZoom on line 316.  This is different from the
> idea of a "minZoom" that to me expresses a limit on zooming.
>
> Does this seem like a reasonable solution?

I think so. Option proliferation is annoying, but less annoying than
not being able to get things working the way you want.
This seems like the right solution to me.

> It looks like philipn has been trying a similar solution here:
>
> https://github.com/philipn/olwidget/commit/5236d058e01c16dc35a4e416aa...
Reply all
Reply to author
Forward
0 new messages