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

[Setup] Add previews to samples #315

Merged
merged 2 commits into from
Dec 11, 2023
Merged

[Setup] Add previews to samples #315

merged 2 commits into from
Dec 11, 2023

Conversation

CalebRas
Copy link
Contributor

@CalebRas CalebRas commented Dec 8, 2023

Description

This PR adds a preview to the top level view for each sample. Samples that used navigation related components, such as a toolbar, had to be wrapped in a NavigationView to work correctly. Some samples also required some small changes for their preview to compile. Those special cases can be found in the second commit of this PR.

Note: With how on-demand resource are set up in this project, they do not work with previews and cause them to crash. As a result, The samples that use on-demand resource were not given a preview.

List of Omitted Samples (as of creation)

  1. Add custom dynamic entity data source
  2. Add feature layers
  3. Add raster from file
  4. Animate 3D graphic
  5. Animate images with image overlay
  6. Augment reality to show tabletop scene
  7. Change camera controller
  8. Display dimensions
  9. Display map from mobile map package
  10. Display scene from mobile scene package
  11. Find route in mobile map package
  12. Find route in transport network
  13. Geocode offline
  14. Identify raster cell
  15. Show device location with NMEA data sources
  16. Show mobile map package expiration date
  17. Show viewshed from geoelement in scene
  18. Style features with custom dictionary
  19. Style symbols from mobile style file

Linked Issue(s)

  • swift/issues/4845

How To Test

  • Verify the preview in the top level view of each sample works as expected.
  • Verify the samples that were additionally modified work as expected.

@CalebRas CalebRas self-assigned this Dec 8, 2023
@CalebRas CalebRas requested review from a team, rolson and dfeinzimer and removed request for a team December 8, 2023 21:50
@CalebRas CalebRas merged commit 8ebfb84 into main Dec 11, 2023
1 check passed
@CalebRas CalebRas deleted the Caleb/Update-AddPreviews branch December 11, 2023 22:06
@yo1995 yo1995 restored the Caleb/Update-AddPreviews branch December 12, 2023 20:05
@yo1995 yo1995 deleted the Caleb/Update-AddPreviews branch December 12, 2023 20:09
@yo1995
Copy link
Collaborator

yo1995 commented Dec 12, 2023

Because the branch was accidentally merged into main branch, we need to merge the changes into v.next as well.

  • I don't think it is a big problem to have them on main for now as we eventually will have these changes on main some time in the next release, so there is no need to rollback these changes.
  • I'll create another PR to cherry pick the most recent 2 commits from main to v.next, if people agree.
  • Another way would be restore the Caleb/Update-AddPreviews branch and open another PR against v.next. This would mess up the commit history a bit, but won't be a huge problem? 🤔
  • I'll add you to review that PR shortly.
    image

thoughts? @dfeinzimer and @rolson

@dfeinzimer
Copy link
Contributor

thoughts?

Maybe it would be cleanest to revert and restore/reopen the PR against v.next?

yo1995 added a commit that referenced this pull request Dec 13, 2023
Revert the changes that were accidentally added
to main branch in #315.
@CalebRas CalebRas restored the Caleb/Update-AddPreviews branch December 13, 2023 21:46
@CalebRas CalebRas deleted the Caleb/Update-AddPreviews branch December 14, 2023 19:41
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.

None yet

4 participants