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

[Links] Show external-link icon where applicable #768

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

meissadia
Copy link
Collaborator

@meissadia meissadia commented Jun 25, 2024

Close #765

Note: May result in unwanted wrapping of icon. Example below (the regulation link):
Screenshot 2024-06-25 at 1 35 15 PM

Changes

  • Adds the external-link icon automatically when we determine it's a non-cfgov link
  • Overrides DS Notification styles to ensure icon colors consistently match link status-based (visited, hover) styles

@meissadia
Copy link
Collaborator Author

meissadia commented Jun 28, 2024

@natalia-fitzgerald Please follow-up with devs on how we want to handle external links where the icon wraps to a new line.

One potential solution is to use the Link component's noWrap property, which would keep the link text + icon together, but looks like it could introduce it's own wrapping oddities. Example screenshot below:

Description Screenshot
Without noWrap (allow link text to wrap) Screenshot 2024-06-28 at 2 47 16 PM
With noWrap (prevent link text from wrapping) Screenshot 2024-06-28 at 12 36 33 PM

@natalia-fitzgerald
Copy link

@meissadia
Can you show the same two screenshots at mobile? I do worry that forcing no wrap could introduce even further oddities at smaller screen widths.

@meissadia
Copy link
Collaborator Author

@meissadia Can you show the same two screenshots at mobile? I do worry that forcing no wrap could introduce even further oddities at smaller screen widths.

@natalia-fitzgerald On large mobile (width: 425px)

Regular No wrap
regular nowrap

@natalia-fitzgerald
Copy link

@meissadia
Would it be possible to push the no wrap solution to a dev server so that I can click around and see how that approach looks across a few instances?

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.

[Links] Show external link icon where applicable
2 participants