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

[VPS-30] Thumbnail Images #228

Merged
merged 10 commits into from
Aug 31, 2024
Merged

[VPS-30] Thumbnail Images #228

merged 10 commits into from
Aug 31, 2024

Conversation

harbassan
Copy link
Contributor

Describe the issue

Thumbnail images use iframes that load really slowly and dont show the correct content due to the way the routing has changed.

Describe the solution

Replaced iframes with dynamically generated svg elements. They use a seperate resolver structure to map the scene components to various elements that are placed within a single svg, which means they are completely resizable and look the same at all sizes, which is good for our use case.

New Thumnail:
image

Risk

No risks

Definition of Done

  • Code peer-reviewed
  • Wiki Documentation is written and up to date
  • Unit tests written and passing
  • Integration tests written and passing
  • Continuous Integration build passing
  • Acceptance criteria met
  • Deployed to production environment

Reviewed By

Who reviewed your PR - for commit history once merged

Copy link
Member

@wjin-lee wjin-lee left a comment

Choose a reason for hiding this comment

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

lgtm - minor nits

@@ -37,76 +37,17 @@ exports[`ListContainer component snapshot test 1`] = `
<div
className="MuiBox-root MuiBox-root-2"
>
Copy link
Member

Choose a reason for hiding this comment

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

SO MUCH BETTER

@harbassan harbassan merged commit 20b6cee into master Aug 31, 2024
6 checks passed
@JordanBlenn JordanBlenn deleted the thumbnails branch August 31, 2024 04:50
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