bloom-housing / ui-seeds

Shared user interface components for Bloom affordable housing system
Apache License 2.0
1 stars 1 forks source link

feat: add Link component #48

Closed jaredcwhite closed 11 months ago

jaredcwhite commented 1 year ago

Issue Overview

This PR addresses #24

Description

Adds a <Link> component which can be used throughout an application (instead of, say, directly using Next's <Link> component). In addition, in a future scenario where there's a prose section, adds an underlined link style there too.

How Can This Be Tested/Reviewed?

Storybook: https://deploy-preview-48--storybook-ui-seeds.netlify.app/?path=/story/actions-link--basic-link

Checklist:

netlify[bot] commented 1 year ago

Deploy Preview for storybook-ui-seeds ready!

Name Link
Latest commit a572e34b736bdb625216fff132533c5844304f25
Latest deploy log https://app.netlify.com/sites/storybook-ui-seeds/deploys/64d6a4787da6b800086e7279
Deploy Preview https://deploy-preview-48--storybook-ui-seeds.netlify.app
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

slowbot commented 1 year ago

@jaredcwhite The external link variant, which should support opening in a new tab needs updates to make it accessible. I think you can add it as a sr only label or with aria

https://codersblock.com/blog/external-links-new-tabs-and-accessibility/ https://medium.com/@svinkle/why-let-someone-know-when-a-link-opens-a-new-window-8699d20ed3b1

jaredcwhite commented 1 year ago

@slowbot Ah, good to know. I'll update this and the Button version of links

emilyjablonski commented 1 year ago

@slowbot One of the links you shared indicates WCAG recommends not opening links in a new tab for a11y - any thoughts on that?

jaredcwhite commented 1 year ago

@emilyjablonski Hmm, I wonder if it's context specific? That W3C notice contains the following about when it's better to open a new tab/window:

Opening a page containing context-sensitive information, such as help instructions, or an alternate means of completing a form, such as a calendar-based date picker, will significantly disrupt a multi-step workflow, such as filling in and submitting a form, if the page is opened in the same window or tab.

So…maybe if you're on a listing and you click an apply button/link to an external website, it shouldn't open a new window because it's the next logical step…but if you're in the middle of a form or search process and there's an external link to some more context, it should open a new window?

(If we agree, I should add a toggle so target="_blank" is optional.)

emilyjablonski commented 1 year ago

^ That sounds perfect! Agreed

jaredcwhite commented 1 year ago

All right, I've updated the Button & Link components with a new newWindowTarget prop with code comment "Set external link target if it meets accessibility criteria" — presumably we can include that accessibility criteria for when it should load a new window or not in the zeroheight docs.

slowbot commented 1 year ago

@jaredcwhite This approach looks good to me!

github-actions[bot] commented 11 months ago

:tada: This PR is included in version 1.10.0 :tada:

The release is available on:

Your semantic-release bot :package::rocket: