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

fix: turn off cleanupIds for svgo loader #268

Merged
merged 4 commits into from
Jul 9, 2024

Conversation

LeahMarieBush
Copy link
Contributor

@LeahMarieBush LeahMarieBush commented Jul 8, 2024

🎟️ Asana Task


Description

There is a problem on safari dev portal where a svg that is used as a background image is blurry. To fix this, I used the svg inline instead of as a background image. That worked in most places but on the hcp page, the svg was getting cut off. This was because the title svg being used was different from other products. This svg had a clip-path and the other product svgs do not. Since the new blurry svg and the title svg have a clip-path, there became a problem. Our svgo settings currently minify the ids on svgs to reduce size. However, if you have two svgs in the same spot, this can become a problem. The algorithm svgo uses is very simple and assigns the ids as a, b, c, ... Because of this, both svgs had a clip-path with an id of 'a' which caused the second svg to get cut off incorrectly.

This PR fixes this problem. Certain plugins are installed as default for svgo, one of them being cleanupIds. This PR changes the settings for that plugin so the ids are no longer minified and we don't run into this problem anymore. There are more details about this setting at the docs and this github issue. Also, this is being used in this dev-portal PR

Screenshots

Before
Screenshot 2024-07-09 at 11 31 28 AM
After
Screenshot 2024-07-09 at 11 30 03 AM

Testing

  1. Go to dev portal preview
  2. Inspect the svgs on the 'Try cloud for free' component
  3. Verify that the clip-paths are using the correct ids instead of 'a', 'b', 'c', etc.

PR Checklist 🚀

  • Conduct thorough self-review.
  • Add or update tests as appropriate.
  • Write a useful description (above) to give reviewers appropriate context.
  • 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 Jul 8, 2024

🦋 Changeset detected

Latest commit: 2bdb0be

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

This PR includes changesets to release 1 package
Name Type
@hashicorp/platform-nextjs-plugin Patch

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

@LeahMarieBush LeahMarieBush added the release:canary Publish a canary release label Jul 8, 2024
Copy link
Contributor

github-actions bot commented Jul 9, 2024

📦 Canary Packages Published

Latest commit: 2bdb0be

Published 1 packages

@hashicorp/platform-nextjs-plugin@5.1.1-canary-202469155816

npm install @hashicorp/platform-nextjs-plugin@canary

@LeahMarieBush LeahMarieBush marked this pull request as ready for review July 9, 2024 16:33
@LeahMarieBush LeahMarieBush requested review from a team and rmainwork and removed request for a team July 9, 2024 16:33
@LeahMarieBush LeahMarieBush merged commit 415eb8f into main Jul 9, 2024
3 checks passed
@LeahMarieBush LeahMarieBush deleted the leah/fix/svgo-loader-settings branch July 9, 2024 19:02
@github-actions github-actions bot mentioned this pull request Jul 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release:canary Publish a canary release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants