CONSOLE-4988: add resizable column feature to tables under workloads option#16152
CONSOLE-4988: add resizable column feature to tables under workloads option#16152vikram-raj wants to merge 8 commits intoopenshift:mainfrom
Conversation
openshift-ci-robot
added
the
jira/valid-reference
|
@vikram-raj: This pull request references CONSOLE-4988 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the spike to target the "4.22.0" version, but no target version was set. DetailsIn response to this:
Instructions 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 openshift-eng/jira-lifecycle-plugin repository. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: vikram-raj The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
bot
added
the
approved
|
@vikram-raj: This pull request references CONSOLE-4988 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the spike to target the "4.22.0" version, but no target version was set. DetailsIn response to this:
Instructions 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 openshift-eng/jira-lifecycle-plugin repository. |
📝 WalkthroughWalkthroughThis pull request introduces resizable column width functionality across multiple data table components in the OpenShift Console frontend. The changes standardize column management by integrating a ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Comment Tip CodeRabbit can generate a title for your PR based on the changes.Add |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
frontend/public/components/configmap.tsx (1)
82-139: Extract the resize-column wiring into a shared helper.This exact
useColumnWidthSettings(model)+getResizableProps(id)+{ columns, resetAllColumnWidths }pattern now exists injob.tsx,hpa.tsx,cron-job.tsx,secret.tsx,replicaset.jsx, andreplication-controller.jsxtoo. Centralizing it in the data-view layer would reduce drift the next time the resize contract changes.As per coding guidelines, "Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/public/components/configmap.tsx` around lines 82 - 139, The repeated pattern in useConfigMapsColumns (useColumnWidthSettings(ConfigMapModel) + getResizableProps(id) + returning { columns, resetAllColumnWidths }) should be extracted into a shared helper in the data-view layer; implement a hook (e.g., useResizableColumns or buildResizableColumns) that accepts the Model and a plain column descriptor array (titles, ids, sorts, props) and returns the final columns with resizableProps injected and resetAllColumnWidths, then replace useConfigMapsColumns, job.tsx, hpa.tsx, cron-job.tsx, secret.tsx, replicaset.jsx and replication-controller.jsx to call the new helper instead of duplicating getResizableProps and resetAllColumnWidths usage (refer to useConfigMapsColumns, useColumnWidthSettings, getResizableProps, resetAllColumnWidths, and the local columns variable to locate where to apply the change).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/public/components/job.tsx`:
- Around line 242-248: The column width state is shared because both
useJobsColumns and CronJobJobsComponent call useColumnWidthSettings(JobModel)
which uses referenceForModel(JobModel) as the storage key; change the key to be
surface-specific by extending the call to useColumnWidthSettings with a context
identifier (e.g., 'jobs-page' vs 'cronjob-tab') or modify useColumnWidthSettings
to accept an optional namespace param and include that in the derived storage
key, updating useJobsColumns and CronJobJobsComponent to pass distinct
identifiers so each surface persists widths independently.
---
Nitpick comments:
In `@frontend/public/components/configmap.tsx`:
- Around line 82-139: The repeated pattern in useConfigMapsColumns
(useColumnWidthSettings(ConfigMapModel) + getResizableProps(id) + returning {
columns, resetAllColumnWidths }) should be extracted into a shared helper in the
data-view layer; implement a hook (e.g., useResizableColumns or
buildResizableColumns) that accepts the Model and a plain column descriptor
array (titles, ids, sorts, props) and returns the final columns with
resizableProps injected and resetAllColumnWidths, then replace
useConfigMapsColumns, job.tsx, hpa.tsx, cron-job.tsx, secret.tsx, replicaset.jsx
and replication-controller.jsx to call the new helper instead of duplicating
getResizableProps and resetAllColumnWidths usage (refer to useConfigMapsColumns,
useColumnWidthSettings, getResizableProps, resetAllColumnWidths, and the local
columns variable to locate where to apply the change).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: eb272e75-3f62-40a1-8af0-4363f2c66cd8
📒 Files selected for processing (8)
frontend/packages/console-app/src/components/pdb/PDBList.tsxfrontend/public/components/configmap.tsxfrontend/public/components/cron-job.tsxfrontend/public/components/hpa.tsxfrontend/public/components/job.tsxfrontend/public/components/replicaset.jsxfrontend/public/components/replication-controller.jsxfrontend/public/components/secret.tsx
📜 Review details
🧰 Additional context used
📓 Path-based instructions (1)
**
⚙️ CodeRabbit configuration file
-Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity.
Files:
frontend/public/components/hpa.tsxfrontend/public/components/configmap.tsxfrontend/packages/console-app/src/components/pdb/PDBList.tsxfrontend/public/components/secret.tsxfrontend/public/components/job.tsxfrontend/public/components/replication-controller.jsxfrontend/public/components/cron-job.tsxfrontend/public/components/replicaset.jsx
🧬 Code graph analysis (6)
frontend/public/components/hpa.tsx (1)
frontend/packages/console-app/src/components/data-view/ConsoleDataView.tsx (2)
nameCellProps(250-253)actionsCellProps(262-266)
frontend/packages/console-app/src/components/pdb/PDBList.tsx (3)
frontend/public/components/factory/table.tsx (1)
TableColumn(663-670)frontend/public/components/replicaset.jsx (4)
useTranslation(56-56)useTranslation(203-203)useColumnWidthSettings(204-204)props(296-296)frontend/packages/console-app/src/components/data-view/ConsoleDataView.tsx (2)
nameCellProps(250-253)actionsCellProps(262-266)
frontend/public/components/secret.tsx (4)
frontend/public/components/factory/table.tsx (1)
TableColumn(663-670)frontend/public/module/k8s/types.ts (1)
SecretKind(1035-1039)frontend/public/models/index.ts (1)
SecretModel(509-522)frontend/packages/console-app/src/components/data-view/ConsoleDataView.tsx (2)
nameCellProps(250-253)actionsCellProps(262-266)
frontend/public/components/job.tsx (4)
frontend/public/components/factory/table.tsx (1)
TableColumn(663-670)frontend/public/module/k8s/types.ts (1)
JobKind(511-522)frontend/public/models/index.ts (1)
JobModel(343-358)frontend/packages/console-app/src/components/data-view/ConsoleDataView.tsx (2)
nameCellProps(250-253)actionsCellProps(262-266)
frontend/public/components/replication-controller.jsx (2)
frontend/public/components/replicaset.jsx (4)
useColumnWidthSettings(204-204)columns(206-271)tableColumnInfo(142-150)props(296-296)frontend/packages/console-app/src/components/data-view/ConsoleDataView.tsx (2)
nameCellProps(250-253)actionsCellProps(262-266)
frontend/public/components/cron-job.tsx (8)
frontend/public/components/factory/table.tsx (1)
TableColumn(663-670)frontend/packages/console-dynamic-plugin-sdk/src/extensions/console-types.ts (1)
TableColumn(309-315)frontend/public/components/RBAC/role.jsx (13)
useTranslation(205-205)useTranslation(247-247)useTranslation(301-301)useTranslation(394-394)useTranslation(424-424)useTranslation(444-444)useTranslation(489-489)columns(248-248)columns(446-446)tableColumnInfo(45-45)props(250-250)props(293-295)props(445-445)frontend/public/components/replication-controller.jsx (4)
useColumnWidthSettings(220-222)columns(224-289)tableColumnInfo(159-167)props(315-315)frontend/public/models/index.ts (1)
CronJobModel(777-792)frontend/public/components/namespace.jsx (4)
columns(350-350)columns(704-704)columns(734-734)props(860-860)frontend/public/components/service-monitor.jsx (2)
columns(152-152)props(151-151)frontend/packages/console-app/src/components/data-view/ConsoleDataView.tsx (2)
nameCellProps(250-253)actionsCellProps(262-266)
🔇 Additional comments (5)
frontend/packages/console-app/src/components/pdb/PDBList.tsx (5)
1-23: Imports look appropriate for the resizable column feature.The new imports (
nameCellProps,useColumnWidthSettings) align with the established pattern from other list components. Organization and paths are correct.
78-155: Clean implementation following the established resizable column pattern.The hook structure mirrors other list components (ReplicaSet, ReplicationController), which is good for maintainability and consistency across the console. The explicit return type annotation aids readability.
One minor note: the
useMemodepends ongetResizableProps(line 152). This is correct assuminguseColumnWidthSettingsreturns a stable reference forgetResizableProps. Based on the shared snippets showing identical usage elsewhere, this appears to be the expected contract.
157-175: Component correctly wires the resizable column feature.The destructuring and prop passing to
ConsoleDataViewfollows the expected pattern. The combination ofhideColumnManagementwithisResizableprovides column resizing without the full column management UI—reasonable for this use case.
35-76: Row data mapping correctly applies cell-level props.The separation between column-level props (in
usePDBColumns) and row-cell props (here usinggetNameCellPropsandactionsCellProps) maintains clean architecture. The sticky behavior for name and actions columns is properly preserved at both levels.
25-33: Column ID definitions are well-structured.Using a centralized
tableColumnInfoarray for column IDs prevents typos and ensures consistency between column definitions and row cell mappings. Good pattern.
| const useJobsColumns = (): { | ||
| columns: TableColumn<JobKind>[]; | ||
| resetAllColumnWidths: () => void; | ||
| } => { | ||
| const { t } = useTranslation(); | ||
| const columns: TableColumn<JobKind>[] = useMemo(() => { | ||
| const { getResizableProps, resetAllColumnWidths } = useColumnWidthSettings(JobModel); | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
hook="$(fd 'useResizableColumnProps' frontend | head -n1)"
if [[ -z "$hook" ]]; then
echo "useResizableColumnProps file not found" >&2
exit 1
fi
printf 'Hook file: %s\n\n' "$hook"
rg -n -C5 'useColumnWidthSettings|referenceForModel|useUserSettings|localStorage|sessionStorage|column.?width|resetAllColumnWidths' "$hook"
printf '\nJobsList call sites:\n\n'
rg -n -C3 'useColumnWidthSettings\(JobModel\)|ListComponent=\{JobsList\}' \
frontend/public/components/job.tsx \
frontend/public/components/cron-job.tsxRepository: openshift/console
Length of output: 4843
Column widths are shared between standalone Jobs and the CronJob jobs tab — confirm this is intentional.
Both the full Jobs page (line 247) and the embedded jobs list in CronJobJobsComponent (cron-job.tsx:298) call useColumnWidthSettings(JobModel). Since the storage key is derived solely from referenceForModel(JobModel), resizing columns in one surface reshapes the other. This unintended coupling may confuse users or violate expectation that each surface has independent preferences. Either scope the key distinctly per surface (e.g., add a context identifier) or document the intentional behavior.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/public/components/job.tsx` around lines 242 - 248, The column width
state is shared because both useJobsColumns and CronJobJobsComponent call
useColumnWidthSettings(JobModel) which uses referenceForModel(JobModel) as the
storage key; change the key to be surface-specific by extending the call to
useColumnWidthSettings with a context identifier (e.g., 'jobs-page' vs
'cronjob-tab') or modify useColumnWidthSettings to accept an optional namespace
param and include that in the derived storage key, updating useJobsColumns and
CronJobJobsComponent to pass distinct identifiers so each surface persists
widths independently.
|
@vikram-raj: The following test failed, say
Full PR test history. Your PR dashboard. 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. |
2 participants
Add a resizable column feature to Secrets, ConfigMaps, CronJobs, Jobs, ReplicaSets, PodDisruptionBudgets, HPA, and ReplicationControllers list pages.
Summary by CodeRabbit