[Git 248] Refactor: Components consolidation core Components#9245
[Git 248] Refactor: Components consolidation core Components#9245codingwolf-at wants to merge 37 commits into
Conversation
…es from web/app/ce
… user interaction
…sUpgrade and remove obsolete components
…w breadcrumb components
…item detail component
…ock component in core
… ExtendedAppHeader, GlobalModals, and SubscriptionPill components
…nboxSourcePill components
…lated components in core
…bar functionality
… and product updates components in core
…date import paths in home and issues sections
…lation options in issue detail components, and introduce new activity helper functions
…troduce it in core filter value input component
…uce new notification components in core
…r PaidPlanUpgradeModal
… for navigation items in core
…s for tour-related files
…ths in profile settings
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
React Doctor found 153 issues in 67 files · 5 errors & 148 warnings · score 69 / 100 (Needs work) · vs Errors
148 warnings
98 more warnings not shown. Reviewed by React Doctor for commit |
…ew workflow-related files in core
…e-related files in core
|
|
… components for gantt-chart functionality
…ntrol and publish components in core
…lling-related files in core
…r workspace members functionality
…new workspace-related files
codingwolf-at
changed the base branch from
preview
to
git-240/hooks-consolidation-core-hooks
…ss various modules
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| // components | ||
| import { LeftResizable } from "./blockResizables/left-resizable"; | ||
| import { RightResizable } from "./blockResizables/right-resizable"; | ||
| import { RightDependencyDraggable, LeftDependencyDraggable } from "../dependency"; |
There was a problem hiding this comment.
React Doctor · react-doctor/no-barrel-import (warning)
This ships extra code to your users & slows page load. Import directly from: "../dependency/blockDraggables/right-draggable", "../dependency/blockDraggables/left-draggable".
Fix → Import from the direct path: import { Button } from './components/Button' instead of ./components
| position={block.position} | ||
| /> | ||
| {/* 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.
|
|
||
| // 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.
| </div> | ||
| <div className="flex gap-2"> | ||
| {/* eslint-disable-next-line jsx-a11y/tabindex-no-positive */} | ||
| <Button variant="secondary" size="lg" onClick={onClose} tabIndex={1}> |
There was a problem hiding this comment.
React Doctor · react-doctor/tabindex-no-positive (warning)
Keyboard users get jumped out of the normal order by a positive tabIndex, so use 0 or -1.
Fix → Use tabIndex={0} (focusable in source order) or tabIndex={-1} (focus only in code).
| onClick={onClose} | ||
| className={getButtonStyling("primary", "lg")} | ||
| // oxlint-disable-next-line jsx-a11y/tabindex-no-positive | ||
| tabIndex={2} |
There was a problem hiding this comment.
React Doctor · react-doctor/tabindex-no-positive (warning)
Keyboard users get jumped out of the normal order by a positive tabIndex, so use 0 or -1.
Fix → Use tabIndex={0} (focusable in source order) or tabIndex={-1} (focus only in code).
| }) | ||
| ); | ||
| // oxlint-disable-next-line eslint-plugin-react-hooks/exhaustive-deps | ||
| }, [projectId, isLastChild, projectListType, handleOnProjectDrop]); |
There was a problem hiding this comment.
React Doctor · react-doctor/exhaustive-deps (warning)
useEffect can run with a stale disableDrag, project, project.logo_props, project.name, disableDrop & 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.
| </div> | ||
| ) : ( | ||
| // oxlint-disable-next-line jsx_a11y/click-events-have-key-events oxlint-disable-next-line jsx_a11y/no-static-element-interactions | ||
| <div className="flex items-center justify-center py-4 text-13 font-medium" onClick={getNextNotifications}> |
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.
| </div> | ||
| ) : ( | ||
| // oxlint-disable-next-line jsx_a11y/click-events-have-key-events oxlint-disable-next-line jsx_a11y/no-static-element-interactions | ||
| <div className="flex items-center justify-center py-4 text-13 font-medium" onClick={getNextNotifications}> |
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.
… workspace notifications
| </Loader> | ||
| )} | ||
| {/* eslint-disable-next-line react/iframe-missing-sandbox oxlint-disable-next-line jsx_a11y/iframe-has-title */} | ||
| <iframe |
There was a problem hiding this comment.
React Doctor · react-doctor/iframe-has-title (warning)
Screen reader users cannot identify this <iframe> because it has no title. Add a title that describes its content.
Fix → Add a descriptive title so screen reader users know what the embedded frame contains.
| </Loader> | ||
| )} | ||
| {/* eslint-disable-next-line react/iframe-missing-sandbox oxlint-disable-next-line jsx_a11y/iframe-has-title */} | ||
| <iframe |
There was a problem hiding this comment.
React Doctor · react-doctor/iframe-missing-sandbox (warning)
An <iframe> with no sandbox is a security hole: the embedded page gets full access to your site.
Fix → Add sandbox="" or a curated value so embedded pages cannot get full access to your site by default.
| import { HomePageHeader } from "@/plane-web/components/home/header"; | ||
| // local imports | ||
| import { StickiesWidget } from "../stickies/widget"; | ||
| import { HomeLoader, NoProjectsEmptyState, RecentActivityWidget } from "./widgets"; |
There was a problem hiding this comment.
React Doctor · react-doctor/no-barrel-import (warning)
This ships extra code to your users & slows page load. Import directly from: "./widgets/loaders/home-loader", "./widgets/empty-states/no-projects", "./widgets/recents".
Fix → Import from the direct path: import { Button } from './components/Button' instead of ./components
| return ( | ||
| <CustomMenu.MenuItem | ||
| // oxlint-disable-next-line react/no-array-index-key | ||
| key={index} |
There was a problem hiding this comment.
React Doctor · react-doctor/no-array-index-as-key (warning)
Your users can see & submit the wrong data when this list reorders or filters, so use a stable id like key={item.id}, not the array index "index".
Fix → Use a stable id from the item, like key={item.id} or key={item.slug}. Index keys break when the list reorders or filters.
| <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.
| // local imports | ||
| import { NotificationEmptyState } from "./empty-state"; | ||
| import { AppliedFilters } from "./filters/applied-filter"; | ||
| import { NotificationSidebarHeader } from "./header"; |
There was a problem hiding this comment.
React Doctor · react-doctor/no-barrel-import (warning)
This ships extra code to your users & slows page load. Import directly from "./header/root".
Fix → Import from the direct path: import { Button } from './components/Button' instead of ./components
| <Header variant={EHeaderVariant.SECONDARY} className="justify-start"> | ||
| {NOTIFICATION_TABS.map((tab) => ( | ||
| // 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.
| <Header variant={EHeaderVariant.SECONDARY} className="justify-start"> | ||
| {NOTIFICATION_TABS.map((tab) => ( | ||
| // 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.
| value={filtersSearchQuery} | ||
| onChange={(e) => setFiltersSearchQuery(e.target.value)} | ||
| // oxlint-disable-next-line jsx_a11y/no-autofocus | ||
| autoFocus={!isMobile} |
There was a problem hiding this comment.
React Doctor · react-doctor/no-autofocus (warning)
autoFocus moves focus on load, which can disrupt screen reader and keyboard users. Remove it and let users choose where to focus.
Fix → Do not use autoFocus. It disorients users on load.
| }) | ||
| ); | ||
| // oxlint-disable-next-line eslint-plugin-react-hooks/exhaustive-deps | ||
| }, [projectId, isLastChild, projectListType, handleOnProjectDrop]); |
There was a problem hiding this comment.
React Doctor · react-doctor/exhaustive-deps (warning)
useEffect can run with a stale disableDrag, project, project.logo_props, project.name, disableDrop & 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.
…onents while updating import paths across various modules
| </Loader> | ||
| )} | ||
| {/* eslint-disable-next-line react/iframe-missing-sandbox oxlint-disable-next-line jsx_a11y/iframe-has-title */} | ||
| <iframe |
There was a problem hiding this comment.
React Doctor · react-doctor/iframe-has-title (warning)
Screen reader users cannot identify this <iframe> because it has no title. Add a title that describes its content.
Fix → Add a descriptive title so screen reader users know what the embedded frame contains.
| </Loader> | ||
| )} | ||
| {/* eslint-disable-next-line react/iframe-missing-sandbox oxlint-disable-next-line jsx_a11y/iframe-has-title */} | ||
| <iframe |
There was a problem hiding this comment.
React Doctor · react-doctor/iframe-missing-sandbox (warning)
An <iframe> with no sandbox is a security hole: the embedded page gets full access to your site.
Fix → Add sandbox="" or a curated value so embedded pages cannot get full access to your site by default.
| import { HomePageHeader } from "@/plane-web/components/home/header"; | ||
| // local imports | ||
| import { StickiesWidget } from "../stickies/widget"; | ||
| import { HomeLoader, NoProjectsEmptyState, RecentActivityWidget } from "./widgets"; |
There was a problem hiding this comment.
React Doctor · react-doctor/no-barrel-import (warning)
This ships extra code to your users & slows page load. Import directly from: "./widgets/loaders/home-loader", "./widgets/empty-states/no-projects", "./widgets/recents".
Fix → Import from the direct path: import { Button } from './components/Button' instead of ./components
| return ( | ||
| <CustomMenu.MenuItem | ||
| // oxlint-disable-next-line react/no-array-index-key | ||
| key={index} |
There was a problem hiding this comment.
React Doctor · react-doctor/no-array-index-as-key (warning)
Your users can see & submit the wrong data when this list reorders or filters, so use a stable id like key={item.id}, not the array index "index".
Fix → Use a stable id from the item, like key={item.id} or key={item.slug}. Index keys break when the list reorders or filters.
| <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.
| <Header variant={EHeaderVariant.SECONDARY} className="justify-start"> | ||
| {NOTIFICATION_TABS.map((tab) => ( | ||
| // 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.
| <Header variant={EHeaderVariant.SECONDARY} className="justify-start"> | ||
| {NOTIFICATION_TABS.map((tab) => ( | ||
| // 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.
|
|
||
| {view?.anchor && publishLink ? ( | ||
| // 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.
|
|
||
| {view?.anchor && publishLink ? ( | ||
| // 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.
| value={filtersSearchQuery} | ||
| onChange={(e) => setFiltersSearchQuery(e.target.value)} | ||
| // oxlint-disable-next-line jsx_a11y/no-autofocus | ||
| autoFocus={!isMobile} |
There was a problem hiding this comment.
React Doctor · react-doctor/no-autofocus (warning)
autoFocus moves focus on load, which can disrupt screen reader and keyboard users. Remove it and let users choose where to focus.
Fix → Do not use autoFocus. It disorients users on load.
…t paths, and streamline workspace notification card structure
…d files in the projects module
| )} | ||
| </div> | ||
| {/* 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.
| )} | ||
| </div> | ||
| {/* 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.
| </div> | ||
| {estimatePoints.length > estimateCount.min && ( | ||
| // 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.
| </div> | ||
| {estimatePoints.length > estimateCount.min && ( | ||
| // 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.
| position={block.position} | ||
| /> | ||
| {/* 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.
| </div> | ||
| <div className="flex gap-2"> | ||
| {/* eslint-disable-next-line jsx-a11y/tabindex-no-positive */} | ||
| <Button variant="secondary" size="lg" onClick={onClose} tabIndex={1}> |
There was a problem hiding this comment.
React Doctor · react-doctor/tabindex-no-positive (warning)
Keyboard users get jumped out of the normal order by a positive tabIndex, so use 0 or -1.
Fix → Use tabIndex={0} (focusable in source order) or tabIndex={-1} (focus only in code).
| onClick={onClose} | ||
| className={getButtonStyling("primary", "lg")} | ||
| // oxlint-disable-next-line jsx-a11y/tabindex-no-positive | ||
| tabIndex={2} |
There was a problem hiding this comment.
React Doctor · react-doctor/tabindex-no-positive (warning)
Keyboard users get jumped out of the normal order by a positive tabIndex, so use 0 or -1.
Fix → Use tabIndex={0} (focusable in source order) or tabIndex={-1} (focus only in code).
| </div> | ||
| ) : ( | ||
| // oxlint-disable-next-line jsx_a11y/click-events-have-key-events oxlint-disable-next-line jsx_a11y/no-static-element-interactions | ||
| <div className="flex items-center justify-center py-4 text-13 font-medium" onClick={getNextNotifications}> |
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.
| </div> | ||
| ) : ( | ||
| // oxlint-disable-next-line jsx_a11y/click-events-have-key-events oxlint-disable-next-line jsx_a11y/no-static-element-interactions | ||
| <div className="flex items-center justify-center py-4 text-13 font-medium" onClick={getNextNotifications}> |
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.
| <div className="mt-8 space-y-5"> | ||
| {sidebarOptions.map((option) => ( | ||
| // oxlint-disable-next-line jsx_a11y/click-events-have-key-events | ||
| <h5 |
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.
2 participants
Description
Moved Components from
apps/web/ce/componentstoapps/web/core/componentsType of Change