DSD-1797: Breadcrumbs custom link prop

7emansell commented 3 months ago

Fixes JIRA ticket DSD-1797.

This PR does the following:

Adds customLinkComponent prop to Breadcrumbs
Adds linkProps prop to breadcrumbsData

How has this been tested?

Locally, and in this Turbine branch

Accessibility concerns or updates

Accessibility Checklist

[x] Checked Storybook's "Accessibility" tab for color contrast and other issues.
[x] The feature works with keyboard inputs including tabbing back and forward and pressing space, enter, arrow, and esc keys.
[x] For hidden text or when aria-live is used, a screenreader was used to verify the text is read.
[x] For features that involve UI updates and focusing on DOM refs, focus management was reviewed.
[x] The feature works when the page is zoomed in to 200% and 400%.

Open Questions

customLink is resistant to stricter typing but I would like for it not to be any?

Checklist:

[x] I have updated the Storybook documentation and changelog accordingly.
[x] I have added relevant accessibility documentation for this pull request.
[x] All new and existing tests passed.

Front End Review:

[x] Review the Vercel preview deployment once it is ready.

vercel[bot] commented 3 months ago

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
nypl-design-system	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Aug 8, 2024 7:26pm

7emansell commented 3 months ago

Looks good but some documentation update. I haven't tested it beyond that time we tested it together and it was working but I'll try to review it in Turbine or DCF if you have a working branch/vercel preview.

https://nypl-ds-test-b59lbu4si-nypl.vercel.app/components/breadcrumbs!

7emansell commented 3 months ago

I feel like customLink is clearer to me as a dev than as (which even as a Chakra convention is slightly confusing) but I agree either could work, so happy to change it!

bigfishdesign13 commented 3 months ago

@7emansell, I don't think as, in this case, is appropriate. As Jackie noted, it doesn't feel right as a component level prop name. Something like linkAs would be better. That said, I am leaning toward the other naming convention. But I think even that could be better. customLinkType or customLinkComponent feels better to me.

Cc @jackiequach @EdwinGuzman

NYPL / nypl-design-system