helpscout / hsds-react

Help Scout Design System (HSDS) — React Component Library
MIT License
86 stars 17 forks source link

HSDS-266 Attachment & AttachmentList update #1073

Closed plbabin closed 2 years ago

plbabin commented 2 years ago

Feature

To support some work happening on the mailbox, we had to update the UI of the attachment & attachment list plugin. This component has not been touch for over 3 years, we took the time to rewrite it as a functional component. All that while staying retro compatible with the API and with the way Provider was use in some places.

CleanShot 2022-06-07 at 16 22 42

CleanShot 2022-06-07 at 16 19 59@2x

html cleanup

We remove the usage of Inline for the grid layout of the AttachmentList. We now rely on flexbox directly on Attachment children.

Broken image

The old UI was just displaying a broken image from the browser. Now we detect that there was an error while loading the image and flip the UI to a grey background component. That ensure that the file name will not overflow out of the container. We also wrap the component with a tooltip.

isRemovable

It's not possible to hide the remove button with this props. By default it will be visible.

Download All

We add the possibility of displaying a download all button in preview mode.

Focus state

We add a focus state for the attachment when there is an url attach to it. User will be able to tab through both attachment and the remove button.

React testing library

Re rewrote all test with RTL

cloudflare-pages[bot] commented 2 years ago

Deploying with  Cloudflare Pages  Cloudflare Pages

Latest commit: 1dea3bd
Status: âœ…  Deploy successful!
Preview URL: https://e812769c.hsds-react.pages.dev
Branch Preview URL: https://hsds-266-attachment-update.hsds-react.pages.dev

View logs

kyliedunkley commented 2 years ago

when I tab through it goes to the x before the image/block in all of them except the last. I am pretty sure that may be a constraint of storybook but wanted to check :)

plbabin commented 2 years ago

when I tab through it goes to the x before the image/block in all of them except the last. I am pretty sure that may be a constraint of storybook but wanted to check :)

this is when there is no url on the attachment (so they are not "clickable"). This is a scenario that will 99.5% never exists in prod

kyliedunkley commented 2 years ago

ooo gotcha then I will go and approve!

luketlancaster commented 2 years ago

@plbabin pulled down the beta and was testing it out in chat. Only thing that looks off is that we're defaulting the Download All button to show, so if we ever end up updating hsds in hs-app that'll start showing up (but since it's not wired up nothing happens when you click). I think we should default to not showing that button, and having to "enable" it in new uses of this component: thoughts?

CleanShot 2022-06-15 at 09 52 15

luketlancaster commented 2 years ago

oh hm...actually thinking about this, I think we show the downloadall by default in the other uses of this component so I don't think we should switch that behavior đŸ€”

I guess we'll just add in the prop to hs-app if we ever get around to updating hsds over there? Hmmm

plbabin commented 2 years ago

oh hm...actually thinking about this, I think we show the downloadall by default in the other uses of this component so I don't think we should switch that behavior đŸ€”

I guess we'll just add in the prop to hs-app if we ever get around to updating hsds over there? Hmmm

yeah that make sense, 7bce1786 contain the update. I keep the download all visible by default on the list theme (preview is turned off by default)

luketlancaster commented 2 years ago

This looks good to me now, gonna run another beta release to verify things still are looking good. If so we can get this thing shipped!