feat: close dropdown on item click #111
Conversation
👷 Deploy request for public-github-stats pending review.Visit the deploys page to approve it
|
|
Instead of writing an util to close the dropdown, which we have to remember to add on each new dropdown, I'd probably go with a generic Dropdown component that handles the logic by itself. |
* main: fix: profile card should have more width DevLeonardoCommunity#112 (DevLeonardoCommunity#113)
This reverts commit dd2c0b1.
|
@Balastrong this refactoring has been quite a journey! At the end of which I am not sure it's worth to have it, because It added a level of complexity to the code base. Have a look and tell me what you think. Anyway it's been interesting to implement it for the sake of learning. |
src/components/Dropdown.tsx
Outdated
| @@ -0,0 +1,62 @@ | |||
| import { FC, ReactElement } from "react"; | |||
|
|
|||
| export type DropdownProps = React.PropsWithChildren<{ | |||
There was a problem hiding this comment.
Do you really need React.PropsWithChildren?
src/components/Dropdown.tsx
Outdated
| items: (React.HTMLProps<HTMLLIElement> & { | ||
| ["data-testid"]?: string; | ||
| onClick?: () => void; | ||
| renderItem: string | ReactElement; |
There was a problem hiding this comment.
With ReactElement you can remove string
| renderItem: string | ReactElement; | |
| renderItem: ReactElement; |
src/components/Dropdown.tsx
Outdated
| align?: "dropdown-end"; | ||
| renderButton: ReactElement; | ||
| items: (React.HTMLProps<HTMLLIElement> & { | ||
| ["data-testid"]?: string; |
There was a problem hiding this comment.
I don't think you need the square brackets here (and the other places where data-testid is used in ThemeSelector.jsx)
src/components/Dropdown.tsx
Outdated
| {items.map((item, index) => { | ||
| const { onClick, renderItem, ...liProps } = item; |
There was a problem hiding this comment.
If you don't need item, you can already destructure it in the loop
| {items.map((item, index) => { | |
| const { onClick, renderItem, ...liProps } = item; | |
| {items.map(({ onClick, renderItem, ...liProps }, index) => { |
* main: fix: icon open PR (DevLeonardoCommunity#114)
src/components/Dropdown.tsx
Outdated
| items: (HTMLProps<HTMLLIElement> & { | ||
| "data-testid"?: string; | ||
| onClick?: () => void; | ||
| renderItem: ReactElement; |
There was a problem hiding this comment.
Sorry I was wrong here, you can remove string only if you replace ReactElement with ReactNode.
With ReactNode you don't need to wrap in <span> the strings.
Added the functionality of closing the dropdown as soon as one of its item is clicked.
There are few repetitions in the code, though:
one is about the export dropdown, which can be resolved creating an
ExportDropdownButtoncomponent.The second one is about the use of the
closeDropdownOnItemClickutil function, which is attached to the dropdown anytime we use it. If we decide to have this functionality throughout the app, maybe a component wrapper could be helpful?@Balastrong what do you think about these considerations?
I leave the PR on draft for now.
Thank you