-
Notifications
You must be signed in to change notification settings - Fork 1k
USWDS: Replaced deprecated xlink:href code with recommended use of href #6165
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Small changes requested.
Also, do you know if we're safe to remove xmlns:xlink
from the svg's as well? Or do we need to keep them?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good. Thanks!
I successfully performed the following tests:
- Confirm all
xlink:href
references have been updated tohref
- Confirm icons still display as expected in Safari, Firefox, Chrome
Can you add a changelog PR for this change?
Additionally, can you update the PR description by adding a "Markup change" section with a note that says users should update their xlink:href
references to href
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good overall, a few minor comments.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@RachelCorsino some small issues related to previous merge conflicts. Can you take a look? Please reach out if anything is unclear.
eefebe2
to
bc7db26
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, added video showing the loader icon to confirm there aren't regressions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No regressions found, sharing for awareness.
loader-icon-2024-11-12.at.9.51.13.AM.mov
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Thanks @RachelCorsino !
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you!
Summary
All
xlink:href
instances have be replaced withhref
within the repo in order to account for the deprecation ofxlink:href
.Breaking change
Indicate if this update is a breaking change with one of the following statements:
This is not a breaking change.
Related issue
Closes #6088
Related pull requests
USWDS-Site: Replaced deprecated xlink:href code with recommended use of href #2926
Preview link
Preview link:
USWDS Storybook Icon
Problem statement
The USWDS repository currently contains numerous instances of xlink:href, which is a deprecated attribute that may cause compatibility issues/errors.
Solution
The USWDSrepository has been updated to use the
href
attribute instead of the deprecatedxlink:href
attribute.Markup Change
Users should update their
xlink:href
references to href.Testing and review
<use>
no longer usesxlink:href
and useshref
instead when inspecting an icon