search

bloom-housing / ui-seeds

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

feat: new Alert, Message, and Toast components #17

Closed jaredcwhite closed 1 year ago

jaredcwhite commented 1 year ago

Addresses #5

Todo:

netlify[bot] commented 1 year ago

Deploy Preview for storybook-ui-seeds ready!

Name Link
Latest commit eeaf9d9df425957afb10506374299af858602633
Latest deploy log https://app.netlify.com/sites/storybook-ui-seeds/deploys/64375caa1c3436000822197b
Deploy Preview https://deploy-preview-17--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 settings.

jaredcwhite commented 1 year ago

This is bit of a weird one, so I wanted to get this out for review before fully finishing up tests/docs. Essentially I tried to come up with a way to have a single base React component serve all three, and equally have base tokens which can optionally get overridden with component-specific tokens (whether that's Alert, or Message, or Toast). Let's talk about if this approach makes sense, or is more confusing than useful. The toasts in particular took a bit of thinking through as I wanted to try to get closer to how they work in other component libraries I've used (aka there can be more than one, and they stack, etc.)

I'm particularly excited about Message as I think we can use that in a lot of places we currently use one-offs like ApplicationStatus. Hopefully it's flexible enough to serve a number of different use cases.

Also I didn't do any work on animation for these, but maybe for a second pass I think showing/hiding alerts and toasts would be nicer if they fade in and out and possibly have a slight bit of motion as well (thought we should be mindful of honoring people's "reduce motion" accessibility preferences).

jaredcwhite commented 1 year ago

Thanks @ludtkemorgan for the feedback, I corrected a few issues that you pointed out.

jaredcwhite commented 1 year ago

I've made a few changes to address feedback. Let me know what you think!

emilyjablonski commented 1 year ago

This is just waiting on a question above, otherwise lgtm!

slowbot commented 1 year ago

@jaredcwhite @emilyjablonski Hope I'm not too late to the party. Would it make sense to group these under the namespace "notification" ?

So instead of "common message" we would use something lilke "notification"

jaredcwhite commented 1 year ago

@slowbot I think Toast, and possibly Alert, could be thought of as notifications, but I don't think Message makes sense. If that gets used in places like the Application Due Date, that doesn't seem like a notification to me and more in the status/tag realm.

github-actions[bot] commented 1 year ago

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

The release is available on:

Your semantic-release bot :package::rocket: