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

implemented TMS in terms of UrlTemplateImageryProvider #3119

Closed
wants to merge 3 commits into from
Closed

implemented TMS in terms of UrlTemplateImageryProvider #3119

wants to merge 3 commits into from

Conversation

sen-lu
Copy link
Contributor

@sen-lu sen-lu commented Oct 23, 2015

due to its async nature, the benefits of implementing TileMapServiceImageryProvider in terms of UrlTemplateImageryProvider as it currently stands are limited
@pjcozzi it might be more complicated than just creating setter functions for the properties in UrlTemplateImageryProvider, since the fileExtension property pulls data from the promise but is also necessary to create the template url for UrlTemplateImageryProvider

…ec floating point comparisons to toBeCloseTo, added async checks
@pjcozzi
Copy link
Contributor

pjcozzi commented Oct 23, 2015

@kring do you have a minute to look at this and advise?

@kring
Copy link
Member

kring commented Oct 25, 2015

Yeah there's not a ton of benefit here, because TMS URLs are so simple to build, and that's the only part that UrlTemplateImageryProvider really helps with. But the code here can be simplified a bit more, making it shorter, more reliable, and faster than the current implementation, at least a little.

To make this simpler, don't set a bunch of fields on TileMapServiceImageryProvider and then call a parameterless buildImageryProvider. Just construct the UrlTemplateImageryProvider directly from local variables in the two places. You should be able to get rid of all the error event, credit, and ready handling.

@pjcozzi
Copy link
Contributor

pjcozzi commented Oct 26, 2015

Part of #2814.

@pjcozzi
Copy link
Contributor

pjcozzi commented Oct 26, 2015

@kring thanks for the review.

@sen-lu sen-lu closed this Oct 30, 2015
@sen-lu sen-lu deleted the urlTemplateImageryProvider branch October 30, 2015 05:48
@sen-lu sen-lu restored the urlTemplateImageryProvider branch October 30, 2015 05:49
@sen-lu sen-lu deleted the urlTemplateImageryProvider branch October 30, 2015 05:50
@sen-lu
Copy link
Contributor Author

sen-lu commented Oct 30, 2015

I'm opening two new pull requests on two renamed branches, one for TileMapServiceImageryProvider (directly addressing the comments here) [#3150] and one for related test fixes [#3151]

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