Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add wrapX support #318

Merged
merged 1 commit into from
Sep 23, 2016
Merged

Conversation

StevenHeinrich
Copy link
Contributor

Allows the wrapX property to be passed into the source. It has only been applied to sources that accept this property based on the OL3 docs. The pass-through property defaults to true, as it is in OL3.

This PR is very similar to #198. The exceptions being lack of ternary operator and only applying the property to sources that have support for it (to avoid confusion and properly match the OL3 docs). Sorry for the repeat PR. But, seeing that #198 was almost a year old and is listed as having merge conflicts... I just chose the simpler path forward.

@juristr
Copy link
Collaborator

juristr commented Sep 22, 2016

Thanks a lot for this PR. Could you please rebase it to resolve the conflicts with the master branch? Then I'll merge it in immediately.

@StevenHeinrich
Copy link
Contributor Author

Conflicts resolved 👍

@juristr juristr merged commit 073eb4d into tombatossals:master Sep 23, 2016
@juristr
Copy link
Collaborator

juristr commented Sep 23, 2016

Awesome, thx! Merged 👌

@thmitchell
Copy link

There is a bug.

wrapX: source.wrapX || true

The use of short-circuit evaluation for the wrapX property will always resolve to true when the possible values are undefined, false, or true.

// wrapX evaluates to true for all of these:
wrapX: undefined || true
wrapX: false || true
wrapX: true || true

Given that... the following seems more appropriate (this actually exists in the Openlayers implementation):

wrapX: options.wrapX !== undefined ? options.wrapX : true

Final thought... why not just let the Openlayers implementation handle the default values if wrapX is undefined? From my brief observations, most of the ol.source.* implementations have default definitions for the wrapX option.

https://github.com/openlayers/ol3/blob/v3.16.0/src/ol/source/xyzsource.js (line 55)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants