Skip to content

feat : Standardize miner table pagination and shared pager UX#1059

Open
carlh7777 wants to merge 3 commits into
entrius:testfrom
carlh7777:feat/miner-pagination
Open

feat : Standardize miner table pagination and shared pager UX#1059
carlh7777 wants to merge 3 commits into
entrius:testfrom
carlh7777:feat/miner-pagination

Conversation

@carlh7777
Copy link
Copy Markdown
Contributor

@carlh7777 carlh7777 commented May 12, 2026

Summary

  • Add URL-backed pagination to the Miner PRs table (prPage / prRows), including a rows-per-page selector.
  • Add shared responsive pager UI.
  • Centralize pagination helpers and hook in TablePagination.tsx.

Type of Change

  • Bug fix
  • New feature
  • Refactor
  • Documentation
  • Other (describe below)

Screenshots

Before

Before.webm

After

After.webm

Checklist

  • New components are modularized/separated where sensible
  • Uses predefined theme (e.g. no hardcoded colors)
  • Responsive/mobile checked
  • Tested against the test API
  • npm run format and npm run lint:fix have been run
  • npm run build passes
  • Screenshots included for any UI/visual changes

@xiao-xiao-mao xiao-xiao-mao Bot added the refactor Code restructuring without behavior change label May 12, 2026
@ventura-oss
Copy link
Copy Markdown
Contributor

Skipping in this review pass — branch has merge conflicts against current test. Please rebase and we'll re-review.

@carlh7777 carlh7777 force-pushed the feat/miner-pagination branch from 60c3676 to cf862ed Compare May 13, 2026 11:48
@carlh7777
Copy link
Copy Markdown
Contributor Author

Hi @ventura-oss

Rebased onto the latest origin/test, resolved the merge conflicts, and force-pushed the updated branch for re-review.

@carlh7777 carlh7777 force-pushed the feat/miner-pagination branch 2 times, most recently from f98a638 to 3b31267 Compare May 14, 2026 08:39
@anderdc
Copy link
Copy Markdown
Collaborator

anderdc commented May 15, 2026

fix conflicts

@carlh7777 carlh7777 force-pushed the feat/miner-pagination branch from 3b31267 to f466de5 Compare May 15, 2026 22:00
@carlh7777
Copy link
Copy Markdown
Contributor Author

Hi @anderdc fixed conflicts

@anderdc
Copy link
Copy Markdown
Collaborator

anderdc commented May 16, 2026

fix conflicts again, this was caused by a PR I just merged

@carlh7777 carlh7777 force-pushed the feat/miner-pagination branch from f466de5 to 0e25363 Compare May 17, 2026 14:11
@carlh7777
Copy link
Copy Markdown
Contributor Author

Hi @anderdc fixed conflicts

@carlh7777 carlh7777 force-pushed the feat/miner-pagination branch from 0e25363 to aee091c Compare May 19, 2026 04:27
@carlh7777 carlh7777 force-pushed the feat/miner-pagination branch from aee091c to c69deb6 Compare May 19, 2026 05:08
@carlh7777
Copy link
Copy Markdown
Contributor Author

Before

image

After

image

Copy link
Copy Markdown
Collaborator

@anderdc anderdc left a comment

Choose a reason for hiding this comment

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

The description says pagination is unified across Miner PRs, Repositories, and Score Breakdown tables, but the diff only migrates the Miner PRs table. Two things to fix:

  1. The TablePagination.tsx top docstring and the PR description reference a Score Breakdown table and scorePage/scoreRows params — neither the table nor those params exist anywhere in the repo. Correct both to describe what the PR actually does: pagination for the Miner PRs table.

  2. Drop the prPageminerPage rename and the legacy-migration code (LEGACY_MINER_PAGE_PARAMS, stripLegacyMinerPaginationParams, and the legacy-key fallbacks in useMinerExplorerPagination). One table uses this param — renaming it only adds migration scaffolding and churns existing deep links for no user-facing gain. Keep prPage.

The rows-per-page selector and numbered/mobile pager are fine — keep those.

@carlh7777
Copy link
Copy Markdown
Contributor Author

@anderdc Fixed all requested review comments.

@anderdc anderdc added enhancement New feature or request and removed refactor Code restructuring without behavior change labels May 22, 2026
Copy link
Copy Markdown
Collaborator

@anderdc anderdc left a comment

Choose a reason for hiding this comment

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

Prior review items are resolved (docstring/description corrected, prPage kept, legacy migration dropped). Two leftovers from the dropped multi-table scope remain:

  1. MinerTableRowsSelect.tsx docstring — "Rows-per-page control for table header toolbars (miner PR / repos tables)" names a repos table this PR doesn't touch, the same phantom reference that was flagged on the TablePagination.tsx docstring. Reword to the Miner PRs table only.

  2. TablePagination.tsx — getMinerExplorerPaging returns rankOffset and pageSize, but the only caller (MinerPRsTable.tsx) destructures neither. Drop both from the return type and body.

Also: default page size drops from 20 to 10 (DEFAULT_MINER_EXPLORER_ROWS) and isn't mentioned in the description. Confirm intended or restore 20.

@carlh7777
Copy link
Copy Markdown
Contributor Author

Hi @anderdc

Fixed the remaining review items

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

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants