Feat/groups refinements #1197
Feat/groups refinements #1197
Conversation
sjaanus
commented
Aug 3, 2022
sjaanus
commented
- Rest of the refinements.
- Mobile/responsive refinements
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
| </IconButton> | ||
| <span> | ||
| <IconButton | ||
| disabled={group?.users.length == 1} |
There was a problem hiding this comment.
Small thing but == in javascript allows type coercion. === will do a strict check on value and type. It's a good pattern to default to ===
| </IconButton> | ||
| <span> | ||
| <IconButton | ||
| disabled={group?.users.length == 1} |
| const useHiddenColumns = (): string[] => { | ||
| const isMediumScreen = useMediaQuery(theme.breakpoints.down('md')); | ||
|
|
||
| return useMemo(() => { |
There was a problem hiding this comment.
What is the value of useMemo here? In my opinion useMemo should only be used for large expensive computation time calculations. If we use it for anything else it's just overhead that doesn't give any tangible benefit and increases the memory footprint.
There was a problem hiding this comment.
Found a way around useMemo
| outline: `${theme.spacing(0.25)} solid ${theme.palette.background.paper}`, | ||
| })); | ||
|
|
||
| const useHiddenColumns = (): string[] => { |
There was a problem hiding this comment.
Could this be reusable and take a list of columns to hide instead of redeclaring this hook?
There was a problem hiding this comment.
Made it into reusable component.
| const useHiddenColumns = (): string[] => { | ||
| const isMediumScreen = useMediaQuery(theme.breakpoints.down('md')); | ||
|
|
||
| return useMemo(() => { |
There was a problem hiding this comment.
Same question as above in regards of tangible benefit.
| onClick={() => setAssignOpen(true)} | ||
| maxWidth="700px" | ||
| Icon={Add} | ||
| permission={UPDATE_PROJECT} |
There was a problem hiding this comment.
This permission will need projectId to work properly
| @@ -107,6 +114,20 @@ interface IProjectGroupViewProps { | |||
| onRemove: () => void; | |||
| } | |||
|
|
|||
| const useHiddenColumns = (): string[] => { | |||
There was a problem hiding this comment.
Opportunity to refactor here. Would be nice to have the same pattern for all lists.
