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

Tile load event #5628

Merged
merged 4 commits into from
Jul 28, 2017
Merged

Tile load event #5628

merged 4 commits into from
Jul 28, 2017

Conversation

lilleyse
Copy link
Contributor

Fixes #5627

@pjcozzi
Copy link
Contributor

pjcozzi commented Jul 14, 2017

@ggetz can you test and review this?

tile.contentReadyPromise.then(function() {
removeFunction();
tileset.tileLoad.raiseEvent(tile);
}).otherwise(removeFunction);
Copy link
Contributor

Choose a reason for hiding this comment

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

Since it looks like this can fail, would it be worth passing an error to the event?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We would probably add a tileFailed event instead, a little more detail here: #5158

Copy link
Contributor

Choose a reason for hiding this comment

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

OK, makes sense!

@ggetz
Copy link
Contributor

ggetz commented Jul 14, 2017

Other than that one comment, looks good.

@lilleyse
Copy link
Contributor Author

I want to keep this open for just a little longer, currently getting feedback on the forum.

@lilleyse
Copy link
Contributor Author

There hasn't been too much activity on the forum post so I think this should go in before the release and if needed tweaked later.

@pjcozzi can you merge?

@pjcozzi pjcozzi merged commit ecfd3cb into master Jul 28, 2017
@pjcozzi pjcozzi deleted the load-event branch July 28, 2017 13:51
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