-
Notifications
You must be signed in to change notification settings - Fork 17
add dropdown #35
add dropdown #35
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
dylsteck
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
made a few small few comments that are more structural(the component itself looks good), I think if you make those changes and also document the component in the README like the other components that this PR should be good to go @MrKevinOConnell
| setTimeout(() => setShowDropdown(false), 200); | ||
| }; | ||
|
|
||
| return ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could you move the actual dropdown component to the molecules folder(eg. UserDropdown.tsx or Dropdown.tsx) -- usually we've had organisms fetch data and pass them down to molecules to follow the atomic design structure
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you change the inline styles to styled components which will also help reduce our bundle size
dylsteck
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm! only other thing I saw was the Vercel deployment failed but other than that this should be good to go @MrKevinOConnell
usersearch.mov