Skip to content
This repository has been archived by the owner on Mar 8, 2023. It is now read-only.

MINOR: tileWrappingEnabled fixes #1767

Merged
merged 1 commit into from
Aug 14, 2020
Merged

MINOR: tileWrappingEnabled fixes #1767

merged 1 commit into from
Aug 14, 2020

Conversation

nzjony
Copy link
Member

@nzjony nzjony commented Aug 13, 2020

  • Added test
  • Log warning when set and projection is spherical

- Added test
- Log warning when set and projection is spherical
@nzjony nzjony requested review from ninok and FraukeF August 13, 2020 15:10
@codecov
Copy link

codecov bot commented Aug 13, 2020

Codecov Report

Merging #1767 into master will increase coverage by 0.01%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1767      +/-   ##
==========================================
+ Coverage   64.32%   64.34%   +0.01%     
==========================================
  Files         283      283              
  Lines       25548    25551       +3     
  Branches     5750     5751       +1     
==========================================
+ Hits        16435    16442       +7     
+ Misses       9113     9109       -4     
Impacted Files Coverage Δ
@here/harp-mapview/lib/MapView.ts 66.62% <100.00%> (+0.33%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1e4316d...afc8f97. Read the comment docs.

@@ -1262,6 +1262,10 @@ export class MapView extends EventDispatcher {
}

set tileWrappingEnabled(enabled: boolean) {
if (this.projection.type === ProjectionType.Spherical) {
logger.warn("Setting this with spherical projection has no affect. Was this intended?");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The more I stumble over the tileWrapping, the more I think it should actually be part of the projection and not of MapView.
This probably goes beyond this fix, but we should at least consider moving it to the projection and see if this would not resolve a lot of this additional and somehow weird and repetitive checks

@nzjony nzjony merged commit 9ea8668 into master Aug 14, 2020
@nzjony nzjony deleted the jsminor138 branch August 14, 2020 08:16
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants