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

feat: use next/image instead of react-inline-svg #1007

Merged
merged 1 commit into from Dec 12, 2023
Merged

Conversation

dstaley
Copy link
Contributor

@dstaley dstaley commented Dec 6, 2023

🎟️ Asana Task
πŸ” Preview Link


Description

This PR replaces instances of ?include and react-inline-svg with next/image or actual inlined SVGs where appropriate. The packages updated in this PR are used on hashicorp.com; other packages likely won't be updated unless they're transitive dependencies of the packages modified in this PR. There may be slight rendering differences due to the switch but this is acceptable given that these components are relatively ancient and should be replaced (all but code-block),

Packages updated:

  • '@hashicorp/react-accordion'
    • Chevron on right side
  • '@hashicorp/react-alert-banner'
    • Close icon
  • '@hashicorp/react-code-block'
    • File icon when filename is rendered
  • '@hashicorp/react-logo-grid'
    • Close icon in popup
  • '@hashicorp/react-split-rich-cta-list'
    • Arrow on right side
  • '@hashicorp/react-tabs'
    • Previous/Next arrows when tabs overflow

PR Checklist πŸš€

Items in this checklist may not may not apply to your PR, but please consider each item carefully.

  • Add Asana and Preview links above.
  • Conduct thorough self-review.
  • Add or update tests as appropriate.
  • Conduct reasonable cross browser testing for both compatibility and responsive behavior (We have a Sauce Labs account for this, if you don't have access, just ask!).
  • Conduct reasonable accessibility review (use the WAS as a guide or an axe browser plugin until we establish more formal checks).
  • Identify (in the description above) and document (add Asana tasks on this board) any technical debt that you're aware of, but are not addressing as part of this PR.

Copy link

changeset-bot bot commented Dec 6, 2023

πŸ¦‹ Changeset detected

Latest commit: 7b91b03

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 6 packages
Name Type
@hashicorp/react-accordion Minor
@hashicorp/react-alert-banner Minor
@hashicorp/react-code-block Minor
@hashicorp/react-logo-grid Minor
@hashicorp/react-split-rich-cta-list Minor
@hashicorp/react-tabs Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copy link

vercel bot commented Dec 6, 2023

The latest updates on your projects. Learn more about Vercel for Git β†—οΈŽ

Name Status Preview Updated (UTC)
react-components βœ… Ready (Inspect) Visit Preview Dec 6, 2023 8:57pm

@dstaley dstaley requested review from a team and pbortnick and removed request for a team December 6, 2023 21:00
@@ -60,7 +60,7 @@
transition: transform 0.4s;
transform: var(--icon-transform);

& svg {
& img {
Copy link
Contributor

@pbortnick pbortnick Dec 6, 2023

Choose a reason for hiding this comment

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

Since this is no longer an svg, the fill and stroke styles below aren't applied and the color is changed. I know you mentioned we are accepting slight rendering differences in the PR description so if this is one of those disregard!

Copy link
Contributor

@pbortnick pbortnick Dec 6, 2023

Choose a reason for hiding this comment

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

Honestly, I think this is fine. Like you mentioned, we are deprecating these anyways.

Copy link
Contributor

@pbortnick pbortnick left a comment

Choose a reason for hiding this comment

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

Nice!

@dstaley dstaley merged commit e49d8f6 into main Dec 12, 2023
7 checks passed
@dstaley dstaley deleted the ds.rm-svg-include branch December 12, 2023 00:58
@hashibot-web hashibot-web mentioned this pull request Dec 12, 2023
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

2 participants