Skip to content

frontend:table: fix the rows-per-page settings persist among views#4668

Open
ChayanDass wants to merge 1 commit intokubernetes-sigs:mainfrom
ChayanDass:fix/rowsperpage
Open

frontend:table: fix the rows-per-page settings persist among views#4668
ChayanDass wants to merge 1 commit intokubernetes-sigs:mainfrom
ChayanDass:fix/rowsperpage

Conversation

@ChayanDass
Copy link
Contributor

@ChayanDass ChayanDass commented Feb 10, 2026 •
edited
Loading

Summary

This PR adds persistent rows per page functionality to tables by storing the selected rows per page value in localStorage . Users can now change the number of rows displayed per page, and their preference is automatically saved and restored on page refresh.

Related Issue

Fixes #3227

Changes

  • Added localStorage persistence for rows per page - When users change the rows per page in any table, the value is automatically saved to localStorage with key tables_rows_per_page.
  • remove useMemo for fetching getTablesRowsPerPage
  • I changed the default perPage value in the URL to match the storeRowsPerPageOptions[0] value, ensuring shared links reflect the actual table state.

Steps to Test

  • Open a cluster resource table (e.g. Workloads → Namespaces).
  • Change the rows-per-page value (e.g. 25 or 50).
  • Reload the page or switch to another view and come back.
  • Confirm the same rows-per-page value is still selected (persisted via localStorage).

Note: We should avoid using an id for reflectInURL, as it makes the URL unnecessarily long, for example:
http://localhost:3000/c/minikube/?headlamp-cluster.overview.events.perPage=25&headlamp-cluster.overview.events.p=3

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Feb 10, 2026
@ChayanDass
Copy link
Contributor Author

ChayanDass commented Feb 10, 2026 •
edited
Loading

HI @illume ..

I think we can remove useSettings here. getTablesRowsPerPage already reads the value from localStorage, and useSettings also persists the same value there.

  const storeRowsPerPageOptions = useSettings('tableRowsPerPageOptions');
  const rowsPerPageOptions = rowsPerPage || storeRowsPerPageOptions;
  const defaultRowsPerPage = getTablesRowsPerPage(rowsPerPageOptions[0]);

Also, we can set reflectInURL to default true, to set the table state values at the URL, which makes it shareable, with preserved table format
.

@k8s-ci-robot k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Feb 10, 2026
@ChayanDass
Copy link
Contributor Author

ChayanDass commented Feb 10, 2026 •
edited
Loading

I removed unnecessary useMemo usage and now pass reflectInURL from parent components. I also noticed that perPage is omitted from the URL when it matches the local default, but defaults can differ between users(if the perpage value already updated and stored in localstorage). This can cause shared links to render different table views, so it may be better to always include perPage in the URL when URL reflection is enabled.

@ChayanDass ChayanDass force-pushed the fix/rowsperpage branch 2 times, most recently from c6eb8bd to cc98748 Compare February 12, 2026 16:48
@illume illume requested a review from Copilot February 12, 2026 20:40
illume
illume reviewed Feb 12, 2026
View reviewed changes
Copy link
Contributor

@illume illume left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you want, you could improve the git message subject body. By including some of the explanation from the PR description and your comment.

illume
illume requested changes Feb 12, 2026
View reviewed changes
Copy link
Contributor

@illume illume left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please add some steps to test in the PR description?

Copilot AI reviewed Feb 12, 2026
View reviewed changes
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds persistent rows-per-page functionality to tables by storing the selected value in localStorage. When users change the number of rows displayed, their preference is saved using the tables_rows_per_page key and automatically restored on future page visits. The implementation uses new helper functions getTablesRowsPerPage and setTablesRowsPerPage to manage localStorage operations.

Changes:

  • Added localStorage persistence for rows per page using helper functions
  • Modified Table component to call setTablesRowsPerPage when pagination changes
  • Removed useMemo optimizations from defaultRowsPerPage, initialState, and state objects
  • Changed ResourceTable default reflectInURL value to always enable URL reflection when id is present

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.

File Description
frontend/src/components/common/Table/Table.tsx Added localStorage persistence call in onPaginationChange; modified URL state handling to always use 'perPage' key; removed useMemo optimizations
frontend/src/components/common/Resource/ResourceTable.tsx Changed default reflectInURL from undefined to template literal using id
frontend/src/components/common/Table/snapshots/Table.ReflectInURL.stories.storyshot Updated snapshot to include perPage=5 parameter in URL
frontend/src/components/common/Table/snapshots/Table.ReflectInURLWithPrefix.stories.storyshot Updated snapshot to include mySuperTable.perPage=5 parameter in URL

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@k8s-ci-robot k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Feb 14, 2026
@ChayanDass
Copy link
Contributor Author

ChayanDass commented Feb 14, 2026

updated, PTAL!

@illume
Copy link
Contributor

illume commented Feb 14, 2026

nit, you could fix the first letter of the subject after the context to be capitalized. Also add some of the Changes part of the PR description into the git message body, and any Why you can think of that may help people.

frontend: table: Add row-per-page persist and update url

illume
illume approved these changes Feb 14, 2026
View reviewed changes
Copy link
Contributor

@illume illume left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! 🎉

@k8s-ci-robot
Copy link
Contributor

k8s-ci-robot commented Feb 14, 2026

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ChayanDass, illume

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 14, 2026
@ChayanDass ChayanDass force-pushed the fix/rowsperpage branch 2 times, most recently from 5e84142 to 1d4f3e7 Compare February 14, 2026 17:24
@ChayanDass
frontend: table: Add row-per-page persist and update URL
c8b8fe2 
- Persist rows-per-page in localStorage (key tables_rows_per_page) when
  the user changes it; value is restored on reload or when switching
  views/tabs.
- Call setTablesRowsPerPage on pagination change so the preference is
  saved from the main Table component.
- Default page size to lowest option (storeRowsPerPageOptions[0]);
  remove getTablesRowsPerPage useMemo and unused import from Table.
- ResourceTable: default reflectInURL to true (no prefix) so page/perPage
  in the URL stay short; using the table id would make the URL too long.

Why: Users expect their chosen table density to persist across
reloads and navigation; storing the value in localStorage avoids
losing the selection and keeps the UI consistent.
@ChayanDass
Copy link
Contributor Author

ChayanDass commented Feb 14, 2026

updated, Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

@illume illume illume approved these changes

@joaquimrocha joaquimrocha Awaiting requested review from joaquimrocha

@yolossn yolossn Awaiting requested review from yolossn

Assignees

No one assigned

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Table: Remember rows per page option

3 participants

@ChayanDass @illume @k8s-ci-robot