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

task/WG-393-Asset-Detail-Pointclouds #296

Merged
merged 10 commits into from
Jan 9, 2025

Conversation

sophia-massie
Copy link
Contributor

@sophia-massie sophia-massie commented Dec 18, 2024

Overview:

Adds asset detail view for point clouds

PR Status:

  • Ready.
  • Work in Progress.
  • Hold.

Related Jira tickets:

Summary of Changes:

  • Adds AssetPointCloud component
  • Adds DOMPurify package to handle Pointclouds urls
  • Strips args and memo logic from AssetRenderer
  • Creates AssetButton component to handle what user do can per each file type and links/onClick

Testing Steps:

  1. npm ci and build
  2. Using prod environment locally, go to Map: test_n0v9_2_delete
  3. Check that point cloud appears in asset detail
  4. Check that View button opens point cloud in potree in a new window
  5. Check that it looks the same as prod here Prod Map: test_n0v9_2_delete
  6. Check other file types (img, movie, geometries, questionnaires) and make sure a View/Download/Add Asset button appears like prod. NOTE: only point cloud button should work atm

UI Photos:

Screenshot 2024-12-18 at 3 33 02 PM

Notes:

Not sure that I love having the asset renderer declared and used in AssetDetail nor how the logic is laid out. Also open to suggestions on the AssetButton and what should be passed to it. 🙏

- Adds AssetPointCloud component
- Adds DOMPurify package to handle Pointclouds urls
- Strips args and memo logic from AssetRenderer
- Creates AssetButton component to handle what user do can per each file type and links/onClick
@sophia-massie sophia-massie changed the title branch task/WG-393-Asset-Detail-Pointclouds task/WG-393-Asset-Detail-Pointclouds Dec 18, 2024
Copy link
Collaborator

@nathanfranklin nathanfranklin left a comment

Choose a reason for hiding this comment

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

LGTM. i have one small comment on variable names and then a suggestion on size of the image/point cloud preview.

react/src/components/AssetDetail/AssetDetail.tsx Outdated Show resolved Hide resolved
Copy link
Collaborator

@nathanfranklin nathanfranklin left a comment

Choose a reason for hiding this comment

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

LGTM 👍

reloading asset repeatedly on page
…etail-Pointclouds' into task/WG-393-Asset-Detail-Pointclouds
@sophia-massie sophia-massie merged commit 69e4163 into main Jan 9, 2025
5 checks passed
@sophia-massie sophia-massie deleted the task/WG-393-Asset-Detail-Pointclouds branch January 9, 2025 18:55
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.

2 participants