Skip to content

Conversation

@rroppolo
Copy link
Collaborator

For #24
Move flash message component into design system and include hook (and pass through HOC that uses the hook too) for better reusability in app components.

@rroppolo rroppolo requested a review from davidmferris March 23, 2020 15:29
@rroppolo rroppolo assigned rsaris and unassigned rsaris Mar 23, 2020
@rroppolo rroppolo requested a review from rsaris March 23, 2020 15:29
Copy link
Contributor

@rsaris rsaris left a comment

Choose a reason for hiding this comment

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

Overall this looks good though I'd point out that this is a pretty new library so there is definitely stuff we still need to bring over and we should try and keep things as generic and reusable as we can. I have left comments where I think we can do that more.

I'll leave it to Dave to comment more on the stories and specs.

Copy link

@davidmferris davidmferris left a comment

Choose a reason for hiding this comment

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

Looking great so far! A few mostly minor comments for ya.

Copy link
Collaborator Author

@rroppolo rroppolo left a comment

Choose a reason for hiding this comment

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

Thanks for the review - back to you @rsaris @davidmferris !

Copy link

@davidmferris davidmferris 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. Nice job @rroppolo

@rsaris rsaris merged commit 02cbdfd into master Mar 25, 2020
@rsaris rsaris deleted the feature/UIDS-24-flash-component branch March 25, 2020 01:46
@rsaris
Copy link
Contributor

rsaris commented Mar 25, 2020

Sorry this review took so long -- @davidmferris and @rroppolo do you want to get a release together and get this working in the Rails app?

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.

4 participants