[GIT-248] Refactor: Components consolidation core Components#9245
[GIT-248] Refactor: Components consolidation core Components#9245codingwolf-at wants to merge 74 commits into
Conversation
…e to web/app/core
…ce to web/app/core
…eb/app/ce to web/app/core
…ne-extensions) from web/app/ce to web/app/core
…ce to web/app/core
…/ce to web/app/core
…/ce to web/app/core
…es from web/app/ce
… components for gantt-chart functionality
…ntrol and publish components in core
…lling-related files in core
…r workspace members functionality
…new workspace-related files
…ss various modules
… workspace notifications
…onents while updating import paths across various modules
…t paths, and streamline workspace notification card structure
…d files in the projects module
… the command palette module
…ss the issues module
…n the editor module
|
Linked to Plane Work Item(s) This comment was auto-generated by Plane |
|
@coderabbitai, review these changes. Most of the files have just been moved, so ignore them and review the rest. We are also cleaning things up as part of this refactor. |
|
✅ Action performedReview finished.
|
| is_pinned: preference?.is_pinned ?? false, | ||
| }; | ||
| return ( | ||
| WORKSPACE_SIDEBAR_DYNAMIC_NAVIGATION_ITEMS_LINKS.filter((item) => { |
There was a problem hiding this comment.
React Doctor · react-doctor/js-combine-iterations (warning)
This loops over your list twice because .filter().map() makes two passes, so do it in one pass with .reduce() or a for...of loop
Fix → Combine .map().filter() style chains into one pass with .reduce() or a for...of loop, so you only loop over the list once
| const sortedNavigationItemsKeys = sortedNavigationItems.map((item) => item.key); | ||
|
|
||
| // oxlint-disable-next-line unicorn/consistent-function-scoping | ||
| const orderNavigationItem = ( |
There was a problem hiding this comment.
React Doctor · react-doctor/prefer-module-scope-pure-function (warning)
orderNavigationItem inside ExtendedAppSidebar uses no local state but is rebuilt on every render, so it wastes work & breaks memoized children. Move it to the top of the file, outside the component.
Fix → Move the function above the component, at the top of the file. It doesn't use local state, so rebuilding it each update is wasted work.
|
|
||
| // helpers | ||
| // oxlint-disable-next-line unicorn/consistent-function-scoping | ||
| const renderIcon = (projectDetails: TProject) => ( |
There was a problem hiding this comment.
React Doctor · react-doctor/prefer-module-scope-pure-function (warning)
renderIcon inside ProjectBreadcrumb uses no local state but is rebuilt on every render, so it wastes work & breaks memoized children. Move it to the top of the file, outside the component.
Fix → Move the function above the component, at the top of the file. It doesn't use local state, so rebuilding it each update is wasted work.
| <CycleAdditionalActions cycleId={cycleId} projectId={projectId} /> | ||
| {showTransferIssues && ( | ||
| // oxlint-disable-next-line jsx_a11y/click-events-have-key-events oxlint-disable-next-line jsx_a11y/no-static-element-interactions | ||
| <div |
There was a problem hiding this comment.
React Doctor · react-doctor/click-events-have-key-events (warning)
Keyboard users can't trigger this click handler because there's no keyboard one, so add onKeyUp, onKeyDown, or onKeyPress.
Fix → Pair onClick with a key handler so keyboard users can trigger it.
| <CycleAdditionalActions cycleId={cycleId} projectId={projectId} /> | ||
| {showTransferIssues && ( | ||
| // oxlint-disable-next-line jsx_a11y/click-events-have-key-events oxlint-disable-next-line jsx_a11y/no-static-element-interactions | ||
| <div |
There was a problem hiding this comment.
React Doctor · react-doctor/no-static-element-interactions (warning)
Screen reader users can't tell this click handler is interactive because it has no role, so add a role or use a button or link.
Fix → Give clickable static elements a role, or use a button or link.
| <ListLoaderItemRow /> | ||
| ) : ( | ||
| // oxlint-disable-next-line jsx_a11y/click-events-have-key-events oxlint-disable-next-line jsx_a11y/no-static-element-interactions | ||
| <div |
There was a problem hiding this comment.
React Doctor · react-doctor/click-events-have-key-events (warning)
Keyboard users can't trigger this click handler because there's no keyboard one, so add onKeyUp, onKeyDown, or onKeyPress.
Fix → Pair onClick with a key handler so keyboard users can trigger it.
| <ListLoaderItemRow /> | ||
| ) : ( | ||
| // oxlint-disable-next-line jsx_a11y/click-events-have-key-events oxlint-disable-next-line jsx_a11y/no-static-element-interactions | ||
| <div |
There was a problem hiding this comment.
React Doctor · react-doctor/no-static-element-interactions (warning)
Screen reader users can't tell this click handler is interactive because it has no role, so add a role or use a button or link.
Fix → Give clickable static elements a role, or use a button or link.
| @@ -237,6 +239,7 @@ export const ListGroup = observer(function ListGroup(props: Props) { | |||
| }) | |||
| ); | |||
| }, [ | |||
There was a problem hiding this comment.
React Doctor · react-doctor/exhaustive-deps (warning)
useEffect can run with a stale handleWorkFlowState, t, handleOnDrop, isExpanded, handleCollapsedGroups & show your users old data.
Fix → Don't blindly add missing dependencies. Read the hook callback first.
Bad:
useEffect(() => {
setCount(count + 1);
}, [count]);
Better:
useEffect(() => {
setCount((currentCount) => currentCount + 1);
}, []);
If the missing value is recreated every render, move it inside the hook or stabilize it before adding it to deps.
| ); | ||
| }, [ | ||
| // oxlint-disable-next-line eslint-plugin-react-hooks/exhaustive-deps | ||
| groupRef?.current, |
There was a problem hiding this comment.
React Doctor · react-doctor/exhaustive-deps (warning)
useEffect won't re-run when groupRef.current changes, since a ref never triggers a redraw.
Fix → Don't blindly add missing dependencies. Read the hook callback first.
Bad:
useEffect(() => {
setCount(count + 1);
}, [count]);
Better:
useEffect(() => {
setCount((currentCount) => currentCount + 1);
}, []);
If the missing value is recreated every render, move it inside the hook or stabilize it before adding it to deps.
| const maxDate = getDate(issue.target_date); | ||
|
|
||
| // oxlint-disable-next-line unicorn/consistent-function-scoping | ||
| const handleEventPropagation = (e: SyntheticEvent<HTMLDivElement>) => { |
There was a problem hiding this comment.
React Doctor · react-doctor/prefer-module-scope-pure-function (warning)
handleEventPropagation inside IssueProperties uses no local state but is rebuilt on every render, so it wastes work & breaks memoized children. Move it to the top of the file, outside the component.
Fix → Move the function above the component, at the top of the file. It doesn't use local state, so rebuilding it each update is wasted work.
| is_pinned: preference?.is_pinned ?? false, | ||
| }; | ||
| return ( | ||
| WORKSPACE_SIDEBAR_DYNAMIC_NAVIGATION_ITEMS_LINKS.filter((item) => { |
There was a problem hiding this comment.
React Doctor · react-doctor/js-combine-iterations (warning)
This loops over your list twice because .filter().map() makes two passes, so do it in one pass with .reduce() or a for...of loop
Fix → Combine .map().filter() style chains into one pass with .reduce() or a for...of loop, so you only loop over the list once
| const sortedNavigationItemsKeys = sortedNavigationItems.map((item) => item.key); | ||
|
|
||
| // oxlint-disable-next-line unicorn/consistent-function-scoping | ||
| const orderNavigationItem = ( |
There was a problem hiding this comment.
React Doctor · react-doctor/prefer-module-scope-pure-function (warning)
orderNavigationItem inside ExtendedAppSidebar uses no local state but is rebuilt on every render, so it wastes work & breaks memoized children. Move it to the top of the file, outside the component.
Fix → Move the function above the component, at the top of the file. It doesn't use local state, so rebuilding it each update is wasted work.
|
|
||
| // helpers | ||
| // oxlint-disable-next-line unicorn/consistent-function-scoping | ||
| const renderIcon = (projectDetails: TProject) => ( |
There was a problem hiding this comment.
React Doctor · react-doctor/prefer-module-scope-pure-function (warning)
renderIcon inside ProjectBreadcrumb uses no local state but is rebuilt on every render, so it wastes work & breaks memoized children. Move it to the top of the file, outside the component.
Fix → Move the function above the component, at the top of the file. It doesn't use local state, so rebuilding it each update is wasted work.
| <CycleAdditionalActions cycleId={cycleId} projectId={projectId} /> | ||
| {showTransferIssues && ( | ||
| // oxlint-disable-next-line jsx_a11y/click-events-have-key-events oxlint-disable-next-line jsx_a11y/no-static-element-interactions | ||
| <div |
There was a problem hiding this comment.
React Doctor · react-doctor/click-events-have-key-events (warning)
Keyboard users can't trigger this click handler because there's no keyboard one, so add onKeyUp, onKeyDown, or onKeyPress.
Fix → Pair onClick with a key handler so keyboard users can trigger it.
| <CycleAdditionalActions cycleId={cycleId} projectId={projectId} /> | ||
| {showTransferIssues && ( | ||
| // oxlint-disable-next-line jsx_a11y/click-events-have-key-events oxlint-disable-next-line jsx_a11y/no-static-element-interactions | ||
| <div |
There was a problem hiding this comment.
React Doctor · react-doctor/no-static-element-interactions (warning)
Screen reader users can't tell this click handler is interactive because it has no role, so add a role or use a button or link.
Fix → Give clickable static elements a role, or use a button or link.
| <ListLoaderItemRow /> | ||
| ) : ( | ||
| // oxlint-disable-next-line jsx_a11y/click-events-have-key-events oxlint-disable-next-line jsx_a11y/no-static-element-interactions | ||
| <div |
There was a problem hiding this comment.
React Doctor · react-doctor/click-events-have-key-events (warning)
Keyboard users can't trigger this click handler because there's no keyboard one, so add onKeyUp, onKeyDown, or onKeyPress.
Fix → Pair onClick with a key handler so keyboard users can trigger it.
| <ListLoaderItemRow /> | ||
| ) : ( | ||
| // oxlint-disable-next-line jsx_a11y/click-events-have-key-events oxlint-disable-next-line jsx_a11y/no-static-element-interactions | ||
| <div |
There was a problem hiding this comment.
React Doctor · react-doctor/no-static-element-interactions (warning)
Screen reader users can't tell this click handler is interactive because it has no role, so add a role or use a button or link.
Fix → Give clickable static elements a role, or use a button or link.
| @@ -237,6 +239,7 @@ export const ListGroup = observer(function ListGroup(props: Props) { | |||
| }) | |||
| ); | |||
| }, [ | |||
There was a problem hiding this comment.
React Doctor · react-doctor/exhaustive-deps (warning)
useEffect can run with a stale handleWorkFlowState, t, handleOnDrop, isExpanded, handleCollapsedGroups & show your users old data.
Fix → Don't blindly add missing dependencies. Read the hook callback first.
Bad:
useEffect(() => {
setCount(count + 1);
}, [count]);
Better:
useEffect(() => {
setCount((currentCount) => currentCount + 1);
}, []);
If the missing value is recreated every render, move it inside the hook or stabilize it before adding it to deps.
| ); | ||
| }, [ | ||
| // oxlint-disable-next-line eslint-plugin-react-hooks/exhaustive-deps | ||
| groupRef?.current, |
There was a problem hiding this comment.
React Doctor · react-doctor/exhaustive-deps (warning)
useEffect won't re-run when groupRef.current changes, since a ref never triggers a redraw.
Fix → Don't blindly add missing dependencies. Read the hook callback first.
Bad:
useEffect(() => {
setCount(count + 1);
}, [count]);
Better:
useEffect(() => {
setCount((currentCount) => currentCount + 1);
}, []);
If the missing value is recreated every render, move it inside the hook or stabilize it before adding it to deps.
| const maxDate = getDate(issue.target_date); | ||
|
|
||
| // oxlint-disable-next-line unicorn/consistent-function-scoping | ||
| const handleEventPropagation = (e: SyntheticEvent<HTMLDivElement>) => { |
There was a problem hiding this comment.
React Doctor · react-doctor/prefer-module-scope-pure-function (warning)
handleEventPropagation inside IssueProperties uses no local state but is rebuilt on every render, so it wastes work & breaks memoized children. Move it to the top of the file, outside the component.
Fix → Move the function above the component, at the top of the file. It doesn't use local state, so rebuilding it each update is wasted work.
Description
Moved Components from
apps/web/ce/componentstoapps/web/core/componentsType of Change