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

View directives on error #242

Merged
merged 3 commits into from
Feb 17, 2016
Merged

View directives on error #242

merged 3 commits into from
Feb 17, 2016

Conversation

jwasilgeo
Copy link
Contributor

@tomwayson please review.

I actually had to do the following in Chrome (48.0.2564.109):
chrome://flags/: Disable 3D software rasterizer (yes, do this)
chrome://settings/: Use hardware acceleration when available (you might also have to un-check this)

Resolves #239
Resolves #240

I tried to split different concerns into different commits:

  • I added new info about the on-error to the patterns page about the other events, but perhaps that wasn't the perfect place to dump in that new info.
  • Also, I don't know of a good way to functionally e2e test the addition of the on-error event to the view directives test pages, so only the site examples currently benefit from this.

Jacob Wasilkowski added 3 commits February 16, 2016 14:35
@jwasilgeo
Copy link
Contributor Author

PS: In an attempt to see these pending changes on my mobile devide, I was not entirely sure if JSAPI was rejecting my scene view promises as suggested by this scene webgl support example. It may be the case that something else is being used in JSAPI docs when navigating to examples on unsupported devices.

@tomwayson tomwayson self-assigned this Feb 16, 2016
@tomwayson
Copy link
Member

If I go to that link on my phone (Chrome on Android), or go directly to the link from the view live sample button on that page, I see the appropriate error messaage (WebGL is not supported on your platform/browser). I also see Chrome's own "Rats! WebGL hit a snag" message. Particularly in the case of the live sample, I don't think this can come from any other code other than than what's in the sample itself. So, ultimately we should be able to reproduce that behaviour in our site.

I'll give this a shot later tonight. In the mean time, if you are able to deploy the updated site to your own fork's gh-pages branch, send the link and I'll test from my phone.

tomwayson added a commit that referenced this pull request Feb 17, 2016
@tomwayson tomwayson merged commit f13713f into master Feb 17, 2016
@tomwayson tomwayson deleted the view-directives-on-error branch February 17, 2016 05:02
@tomwayson
Copy link
Member

@jwasil

excellent work on a tough problem. I tested using chrome remote debugging on my phone and also pushed to my fork's gh-pages, and it seems to me that sometimes (usually the first time?) I load any example page w/ a web scene on my phone, I don't see the error message that I should, but subsequent times I do.

At first I thought maybe we do need $scope.$apply() in the promise's errback, so I tried that and saw all the already in digest nonsense, so I undid that.

I tried a few other things, including setting the view's div to display:none in the errback (as the JSAPI sample does), but I could not reproduce the problem (of not seeing the error message).

Anyway, I think what you have here is the best we can do for now, so I've merged and deployed gh-pages. We'll just have to keep an eye on this and see if anything comes up.

@jwasilgeo
Copy link
Contributor Author

Thanks for the very thorough review and help with investigating this PR. Hopefully this will take care of it, but yes, we may need to revisit one day. Thanks!

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