Skip to content

fix(icon): fix for icons not loading #3454

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

Merged
merged 1 commit into from
Dec 18, 2024
Merged

Conversation

jawinn
Copy link
Collaborator

@jawinn jawinn commented Dec 18, 2024

Description

Icons were not loading on the spectrum-two branch. The require statements in the fetchIconSVG function are not working, but that function no longer exists in main. This is a light touch update so icons appear for the current work, until spectrum-two is back up to date with main.

This updates the default useRef to true in the Icon component, so icons use the spritesheet version. useRef is not set to anything other than the default in the repo.

How and where has this been tested?

Please tag yourself on the tests you've marked complete to confirm the tests have been run by someone other than the author.

Validation steps

  • Icons are loading again -- Icon component workflow and UI
  • Icons are appearing again in other components

To-do list

  • I have read the contribution guidelines.
  • I have updated relevant storybook stories and templates.
  • ✨ This pull request is ready to merge. ✨

Copy link

changeset-bot bot commented Dec 18, 2024

⚠️ No Changeset found

Latest commit: 35f9d74

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

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

Copy link
Contributor

github-actions bot commented Dec 18, 2024

🚀 Deployed on https://pr-3454--spectrum-css.netlify.app

Icons were not loading on the spectrum-two branch. The fetchIconSVG
function is not working (this function no longer exists in main). This
is a light touch update so icons appear, until spectrum-two is back up
to date with main.
@jawinn jawinn force-pushed the jawinn/s2-icon-use-ref branch from d371d1b to 35f9d74 Compare December 18, 2024 21:52
Copy link
Contributor

github-actions bot commented Dec 18, 2024

File metrics

Summary

Total size: 2.74 MB*

🎉 No changes detected in any packages

* Size determined by adding together the size of the main file for all packages in the library.
* Results are not gzipped or minified.
* An ASCII character in UTF-8 is 8 bits or 1 byte.

@cdransf cdransf merged commit 213f61b into spectrum-two Dec 18, 2024
24 checks passed
@cdransf cdransf deleted the jawinn/s2-icon-use-ref branch December 18, 2024 22:53
castastrophe pushed a commit that referenced this pull request Dec 27, 2024
Icons were not loading on the spectrum-two branch. The fetchIconSVG
function is not working (this function no longer exists in main). This
is a light touch update so icons appear, until spectrum-two is back up
to date with main.
castastrophe pushed a commit that referenced this pull request Dec 29, 2024
Icons were not loading on the spectrum-two branch. The fetchIconSVG
function is not working (this function no longer exists in main). This
is a light touch update so icons appear, until spectrum-two is back up
to date with main.
castastrophe pushed a commit that referenced this pull request Dec 29, 2024
Icons were not loading on the spectrum-two branch. The fetchIconSVG
function is not working (this function no longer exists in main). This
is a light touch update so icons appear, until spectrum-two is back up
to date with main.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants