8000 USWDS: Replaced deprecated xlink:href code with recommended use of href by RachelCorsino · Pull Request #6165 · uswds/uswds · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

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

Merged
merged 11 commits into from
Nov 22, 2024

Conversation

RachelCorsino
Copy link
Contributor
@RachelCorsino RachelCorsino commented Oct 30, 2024

Summary

All xlink:href instances have be replaced with href within the repo in order to account for the deprecation of xlink: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 deprecated xlink:href attribute.

Markup Change

Users should update their xlink:href references to href.

Testing and review

  1. Open up local version of USWDS Storybook or preview link
  2. Ensure icons are working as expected
  3. Using dev tools, ensure the <use> no longer uses xlink:href and uses href instead when inspecting an icon

@RachelCorsino RachelCorsino requested review from amyleadem, mejiaj, mahoneycm and cathybaptista and removed request for amyleadem October 31, 2024 19:35
@RachelCorsino RachelCorsino marked this pull request as ready for review October 31, 2024 20:12
Copy link
Contributor
@mahoneycm mahoneycm left a 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?

amyleadem
amyleadem previously approved these changes Nov 1, 2024
Copy link
Contributor
@amyleadem amyleadem left a 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 to href
  • 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?

Copy link
Contributor
@mejiaj mejiaj left a 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.

@RachelCorsino RachelCorsino requested a review from mejiaj November 7, 2024 17:18
Copy link
Contributor
@mejiaj mejiaj left a 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.

@RachelCorsino RachelCorsino requested a review from a team as a code owner November 7, 2024 22:25
@RachelCorsino RachelCorsino requested a review from mejiaj November 7, 2024 23:19
Copy link
Contributor
@mejiaj mejiaj left a 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.

Copy link
Contributor

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

Copy link
Contributor
@mahoneycm mahoneycm left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks @RachelCorsino !

@amyleadem amyleadem requested a review from thisisdano November 14, 2024 19:47
Copy link
Contributor
@thisisdano thisisdano left a comment

Choose a reason for hiding this comment

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

Thank you!

@thisisdano thisisdano merged commit c774a84 into develop Nov 22, 2024
5 checks passed
@thisisdano thisisdano deleted the rc-xlink-deprecated branch November 22, 2024 17:51
@thisisdano thisisdano mentioned this pull request Dec 18, 2024
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.

USWDS - Bug: xlink:href deprecated
5 participants
0