Skip to content
This repository was archived by the owner on Dec 1, 2020. It is now read-only.

chaining promises additional promise#257

Closed
jwasilgeo wants to merge 1 commit intomasterfrom
chaining-promises-example-update
Closed

chaining promises additional promise#257
jwasilgeo wants to merge 1 commit intomasterfrom
chaining-promises-example-update

Conversation

@jwasilgeo
Copy link
Copy Markdown
Contributor

@tomwayson there's no github issue associated with this, but I worked up a small optional fix for behavior I noticed recently in Firefox and IE for the chaining promises example. Check out the commit description for more info. A drawback is that this departs from the JSAPI example. Any thoughts or reasons to forget and deny this?

…moved,

otherwise they don't always enter that particular callback;
(already taken care of within the scene directive's on-create)
@tomwayson
Copy link
Copy Markdown
Member

Do you see this same behavior in the JSAPI sample page in IE/Firefox? If so, best option is to leave as is and notify JSAPI team.

@jwasilgeo
Copy link
Copy Markdown
Contributor Author

Good question. I'm not really seeing it happen (works fine). I can even keep on nesting return view.then(function() { in the sandbox in FF without a hiccup. Sandbox doesn't fire up at all for me in IE11. Meh...maybe this PR isn't worth it for now.

@tomwayson
Copy link
Copy Markdown
Member

Leave it open and revisit after 4.0 is out of beta.

@jwasilgeo jwasilgeo mentioned this pull request May 1, 2016
14 tasks
@jwasilgeo
Copy link
Copy Markdown
Contributor Author

Let's cancel this, it appears to be fine at 4.0 from what I can see in the test page.

@jwasilgeo jwasilgeo closed this May 6, 2016
@jwasilgeo jwasilgeo deleted the chaining-promises-example-update branch May 6, 2016 12:07
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