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

[NO-TICKET] Use the external link component in doc site #2522

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

pwolfert
Copy link
Collaborator

@pwolfert pwolfert commented Jun 7, 2023

Note that we're going to need to refine the component's styles. I'm putting up this PR early so we have a place to discuss changes we might need to make to that component.

Examples of the problem with using it as-is in blocks of text:

Screenshot 2023-06-07 at 11 41 35 AM Screenshot 2023-06-07 at 11 41 24 AM

Summary

How to test

Checklist

  • Prefixed the PR title with the Jira ticket number as [WNMGDS-####] Title or [NO-TICKET] if this is unticketed work.
  • Selected appropriate Type (only one) label for this PR, if it is a breaking change, label should only be Type: Breaking
  • Selected appropriate Impacts, multiple can be selected.

If this is a change to code:

  • Created or updated unit tests to cover any new or modified code
  • If necessary, updated unit-test snapshots (yarn test:unit:update) and browser-test snapshots (yarn test:browser:update)

If this is a change to documentation:

  • Checked for spelling and grammatical errors

Note that we're going to need to refine the component's styles in order for it to work well
Our argument for using a link is that even though it doesn't perform the navigation directly (normal a11y rule for using a button instead of a link), the intent of the element is to perform navigation, which makes it a link. This also allows desktop users to hover over the link and see the address, and it allows some other standard browser behaviors. It is also way easier to style to look like a link, because it is a link. We realized we were going to have to fix the styles anyway because its primary use case is to be within blocks of text.
Copy link
Collaborator

@zarahzachz zarahzachz left a comment

Choose a reason for hiding this comment

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

Looks awesome! And I like that the text wraps when on smaller screens!

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.

None yet

3 participants