refactor: split high-complexity components into focused files#4062
Conversation
kubestellar-prow
bot
added
do-not-merge/work-in-progress
✅ Deploy Preview for kubestellarconsole ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
👋 Hey @Copilot — thanks for opening this PR!
This is an automated message. |
github-actions
bot
removed
the
dco-signoff: no
❌ PR Title Verification FailedYour PR title does not follow the required format. Current title: Required FormatPR titles must start with one of these emoji prefixes:
How to FixEdit your PR title to start with the appropriate emoji. For example:
You can edit the title by clicking the Edit button next to your PR title. This comment was automatically posted by the PR Title Verifier workflow. |
kubestellar-prow
bot
added
the
size/XS
github-actions
bot
added
dco-signoff: yes
- Extract useHoverState hook from SnoozedCards.tsx → hooks/useHoverState.ts - Extract SnoozedItem sub-component → layout/SnoozedItem.tsx - Extract SnoozedRecommendationItem sub-component → layout/SnoozedRecommendationItem.tsx - Extract SnoozedMissionItem sub-component → layout/SnoozedMissionItem.tsx - Extract GitHubCI utilities (formatTimeAgo, loadRepos, saveRepos) → gitHubCIUtils.ts SnoozedCards.tsx: 419 → 150 lines; GitHubCIMonitor.tsx: 541 → 510 lines Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Agent-Logs-Url: https://github.com/kubestellar/console/sessions/c7fe5c37-1522-4aaa-9588-1f0415dd4d26 Co-authored-by: clubanderson <407614+clubanderson@users.noreply.github.com>
kubestellar-prow
bot
added
dco-signoff: no
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
Thanks for your pull request. Before we can look at it, you'll need to add a 'DCO signoff' to your commits. 📝 Please follow instructions in the contributing guide to update your commits with the DCO Full details of the Developer Certificate of Origin can be found at developercertificate.org. The list of commits missing DCO signoff:
DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
kubestellar-prow
bot
added
size/XL
github-actions
bot
added
dco-signoff: yes
clubanderson
removed
the
do-not-merge/work-in-progress
There was a problem hiding this comment.
Pull request overview
Refactors two high-complexity UI areas by extracting inline subcomponents and utilities into dedicated modules, reducing file size and improving reuse/maintainability within the web/ React + TypeScript frontend.
Changes:
- Extracted
useHoverStateinto a reusable hook (web/src/hooks/useHoverState.ts). - Split
SnoozedCardsinline item renderers into dedicated layout components (SnoozedItem,SnoozedRecommendationItem,SnoozedMissionItem). - Moved GitHub CI monitor localStorage + time formatting helpers into a collocated utility module (
gitHubCIUtils.ts).
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| web/src/hooks/useHoverState.ts | Introduces a shared hover-state hook used by the extracted snoozed item components. |
| web/src/components/layout/SnoozedItem.tsx | New extracted component for snoozed card swap items. |
| web/src/components/layout/SnoozedRecommendationItem.tsx | New extracted component for snoozed recommendation items. |
| web/src/components/layout/SnoozedMissionItem.tsx | New extracted component for snoozed mission items. |
| web/src/components/layout/SnoozedCards.tsx | Updated to consume extracted item components and removed inline implementations. |
| web/src/components/cards/workload-monitor/gitHubCIUtils.ts | New utility module for repo persistence and relative-time formatting. |
| web/src/components/cards/workload-monitor/GitHubCIMonitor.tsx | Updated to import helpers from the new utility module. |
| /** | ||
| * Simple hook for tracking hover state with mouse event handlers. | ||
| * Returns isHovered boolean and hoverProps to spread onto an element. | ||
| */ | ||
| export function useHoverState() { | ||
| const [isHovered, setIsHovered] = useState(false) | ||
| return { | ||
| isHovered, | ||
| hoverProps: { | ||
| onMouseEnter: () => setIsHovered(true), | ||
| onMouseLeave: () => setIsHovered(false), |
There was a problem hiding this comment.
useHoverState only wires mouse enter/leave, so hover-only UI (e.g., action buttons) can’t be revealed via keyboard focus and is unreliable on touch. Consider adding focus/blur and pointer enter/leave handlers (and/or exposing a way to force-hover) so consumers can make the same interactions accessible without duplicating logic.
| /** | |
| * Simple hook for tracking hover state with mouse event handlers. | |
| * Returns isHovered boolean and hoverProps to spread onto an element. | |
| */ | |
| export function useHoverState() { | |
| const [isHovered, setIsHovered] = useState(false) | |
| return { | |
| isHovered, | |
| hoverProps: { | |
| onMouseEnter: () => setIsHovered(true), | |
| onMouseLeave: () => setIsHovered(false), | |
| type UseHoverStateOptions = { | |
| /** | |
| * When true, forces the hook to report a hovered state regardless of | |
| * incoming events. Event handlers still update internal state so that | |
| * the hook behaves correctly if this flag is later removed or toggled. | |
| */ | |
| forceHover?: boolean | |
| } | |
| /** | |
| * Simple hook for tracking hover-like state with event handlers. | |
| * Returns isHovered boolean and hoverProps to spread onto an element. | |
| * Supports mouse, pointer, and focus interactions. | |
| */ | |
| export function useHoverState(options?: UseHoverStateOptions) { | |
| const [isHoveredInternal, setIsHoveredInternal] = useState(false) | |
| const isHovered = options?.forceHover ?? isHoveredInternal | |
| const setHoveredTrue = () => setIsHoveredInternal(true) | |
| const setHoveredFalse = () => setIsHoveredInternal(false) | |
| return { | |
| isHovered, | |
| hoverProps: { | |
| // Mouse events (existing behavior) | |
| onMouseEnter: setHoveredTrue, | |
| onMouseLeave: setHoveredFalse, | |
| // Pointer events (better touch / pen support) | |
| onPointerEnter: setHoveredTrue, | |
| onPointerLeave: setHoveredFalse, | |
| // Focus events (keyboard accessibility) | |
| onFocus: setHoveredTrue, | |
| onBlur: setHoveredFalse, |
| > | ||
| <X className="w-3 h-3" /> |
There was a problem hiding this comment.
Icon-only dismiss button is missing an accessible name. Please add an aria-label (and typically mark the icon aria-hidden) so screen readers announce the control meaningfully.
| > | |
| <X className="w-3 h-3" /> | |
| aria-label={t('actions.dismissSnoozedItem')} | |
| > | |
| <X className="w-3 h-3" aria-hidden="true" /> |
| > | ||
| <X className="w-3 h-3" /> |
There was a problem hiding this comment.
Icon-only dismiss button is missing an accessible name. Please add an aria-label (and typically mark the icon aria-hidden) so screen readers announce the control meaningfully.
| > | |
| <X className="w-3 h-3" /> | |
| aria-label="Dismiss snoozed recommendation" | |
| > | |
| <X className="w-3 h-3" aria-hidden="true" /> |
| > | ||
| <X className="w-3 h-3" /> |
There was a problem hiding this comment.
Icon-only dismiss button is missing an accessible name. Please add an aria-label (and typically mark the icon aria-hidden) so screen readers announce the control meaningfully.
| > | |
| <X className="w-3 h-3" /> | |
| aria-label={t('actions.dismissSnoozedMission', 'Dismiss snoozed mission')} | |
| > | |
| <X className="w-3 h-3" aria-hidden="true" /> |
| /** Loads the configured repo list from localStorage, falling back to DEFAULT_REPOS. */ | ||
| export function loadRepos(): string[] { | ||
| try { | ||
| const stored = localStorage.getItem(REPOS_STORAGE_KEY) | ||
| if (stored) { | ||
| const parsed = JSON.parse(stored) | ||
| if (Array.isArray(parsed) && parsed.length > 0) { | ||
| return parsed | ||
| } |
There was a problem hiding this comment.
loadRepos() returns the raw parsed JSON array without validating element types. Since localStorage is user-controllable, consider validating/coercing to a bounded string[] (e.g., filter to strings and cap length) to avoid unexpected values causing downstream fetch/URL issues.
|
|
||
| /** Persists the repo list to localStorage. */ | ||
| export function saveRepos(repos: string[]) { | ||
| localStorage.setItem(REPOS_STORAGE_KEY, JSON.stringify(repos)) |
There was a problem hiding this comment.
saveRepos() writes to localStorage without a try/catch. In environments where storage is unavailable/blocked (privacy mode, disabled storage, SSR/tests), this can throw and crash the component. Wrap the write in a try/catch (and consider no-op fallback) to keep the UI resilient.
| localStorage.setItem(REPOS_STORAGE_KEY, JSON.stringify(repos)) | |
| try { | |
| localStorage.setItem(REPOS_STORAGE_KEY, JSON.stringify(repos)) | |
| } catch { | |
| // Ignore storage errors to keep UI resilient (e.g. privacy mode, SSR/tests) | |
| } |
| export const REPOS_STORAGE_KEY = 'github_ci_repos' | ||
| export const DEFAULT_REPOS = ['kubestellar/kubestellar', 'kubestellar/console'] |
There was a problem hiding this comment.
The module filename gitHubCIUtils.ts uses mixed casing that’s easy to mistype and is inconsistent with common github... casing patterns. Consider renaming to a more conventional filename (e.g., githubCIUtils.ts) to reduce import-path friction on case-sensitive filesystems.
🔄 Auto-Applying Copilot Code ReviewCopilot code review found 5 code suggestion(s) and 2 general comment(s). @copilot Please apply all of the following code review suggestions:
Also address these general comments:
Push all fixes in a single commit. Run Auto-generated by copilot-review-apply workflow. |
kubestellar-prow
bot
deleted the
copilot/auto-qa-split-high-complexity-components-again
branch
|
Thank you for your contribution! Your PR has been merged. Check out what's new:
Stay connected: Slack #kubestellar-dev | Multi-Cluster Survey |
Rebased the type safety PR (#4085) on current main to resolve merge conflicts with the component split (#4062). Also fixed 5 new type errors introduced by the any→unknown change: - kubectlProxy.ts: guard optional waitReason with ?? '' - Marketplace.tsx: narrow unknown data with type guard before access Original changes (from Copilot): - 3 source files: any → unknown/proper types - 62 test files: remove {} as any spreads, use typed props Signed-off-by: Andrew Anderson <andy@clubanderson.com>
…#4098) Rebased the type safety PR (#4085) on current main to resolve merge conflicts with the component split (#4062). Also fixed 5 new type errors introduced by the any→unknown change: - kubectlProxy.ts: guard optional waitReason with ?? '' - Marketplace.tsx: narrow unknown data with type guard before access Original changes (from Copilot): - 3 source files: any → unknown/proper types - 62 test files: remove {} as any spreads, use typed props Signed-off-by: Andrew Anderson <andy@clubanderson.com>
Auto-QA flagged several files exceeding 400+ lines with many hooks. This PR addresses two of the most structurally clear cases by extracting sub-components and utilities into dedicated modules.
📌 Fixes
📝 Summary of Changes
SnoozedCards.tsx: 419 → 150 lines — three inline sub-components and auseHoverStatehook extracted into separate filesGitHubCIMonitor.tsx: 541 → 510 lines — localStorage utilities andformatTimeAgomoved to a collocated utils moduleChanges Made
useHoverState()→web/src/hooks/useHoverState.ts(now reusable)SnoozedItem→web/src/components/layout/SnoozedItem.tsxSnoozedRecommendationItem→web/src/components/layout/SnoozedRecommendationItem.tsxSnoozedMissionItem→web/src/components/layout/SnoozedMissionItem.tsxformatTimeAgo,loadRepos,saveRepos,REPOS_STORAGE_KEY,DEFAULT_REPOS→web/src/components/cards/workload-monitor/gitHubCIUtils.tsChecklist
git commit -s)Screenshots or Logs (if applicable)
N/A — pure refactor, no behavioral changes.
👀 Reviewer Notes
No logic was changed — this is purely mechanical extraction. Each sub-component retains its original props interface and rendering logic verbatim. The
useHoverStatehook is a candidate for broader reuse across the layout layer.