This repository has been archived by the owner on Dec 1, 2020. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 107
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…t); minor documentation changes
…; cleanup on a few other pages
that fixes the issue w/ the web map test/example page not woking not saying it's pretty, and that it's what we should do long term, but it works.
remove $scope.$evalAsync/$apply from webscene slides test/example by - using promise form of esriLoader.require() - see #219 - moving view.then() into new on-load event of scene view directive the on-load event is more consistent w/ v1 map directive's load event see /~https://github.com/Esri/angular-esri-map/blob/1e45fd121a90d0860aaaf3e24f944389fadc4378/src/map/EsriMapController.js#L176-L200 if this seems like a good approach we should add an on-load event to the map view
…ction; commented out failing unit tests
when in doubt, $rootScope.$digest()
Require callback digest
Conflicts: site/app/examples/webscene-slides.js test/webscene-slides.html
added on-load event to scene view directive
…ncorrectly placed on small screens
another (final?) fix for snippets column alignment
I went ahead and just pushed a final commit that also resolves #224 to add the property binding to the examples site. I think that commit also finally fixes the examples site snippets column alignment. |
@jwasil impressive work. I've done a review, and everything looks perfect. Only issue is that I get one failing functional test for popups. Is that expected given the known bug w/ pop ups? If so we should skip it w/ |
Thanks for the review of this big old PR! Yes, that should fail for now with what we know about popups. Do we also want to hide it from the examples page (comment out from appConfig.js?) |
Good idea, let's do that as well. Please push a commit w/ those changes and I'll merge. |
… positioning is changed
Cool, thanks for the feedback. Good to go. |
3 tasks
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
@tomwayson please review. This branch introduces a slew of new test pages, example pages, and e2e tests. This covers the following issues, but there are a few caveats:
926a24b reverses a right float change from #192. I found that on small screens, the snippets div actually gets moved out of place and offset too much to the left when browsing examples. We can revisit a fix for that, if we see what was described in that issue, if the left nav becomes really short again.
And, just for a few housekeeping reminders, this PR includes merged/closed PRs #220 and #221, and is fully concerned with milestone v2-beta.2.