-
-
Notifications
You must be signed in to change notification settings - Fork 471
fix(pagination): allow single page display by updating range condition #1618
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
fix(pagination): allow single page display by updating range condition #1618
Conversation
…tion): allow range function to return single page (fixes themesberg#1048)Update helpers.ts Changed condition from start >= end to start > end to allow range function to return [start] when start equals end. This fixes the pagination component not showing page 1 when there's only one page. Addresses issue themesberg#1048.
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughAdjusts the Pagination helper’s range function to include the end value when start equals end (returning [start] for start == end). Updates inline documentation accordingly. No exported APIs changed. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor UI as Pagination UI
participant Helper as helpers.ts (range)
UI->>Helper: range(start=1, end=1)
Note right of Helper: New behavior: inclusive end when start==end
Helper-->>UI: [1]
Note over UI: UI renders single page "1"
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changesSuggested reviewers
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/ui/src/components/Pagination/helpers.ts (1)
5-11
: Minor doc nit: specify integer inputs to avoid ambiguity with non-integers.Since
Array(end - start + 1)
requires an integer length, consider noting thatstart
andend
are integers (page indices).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
packages/ui/src/components/Pagination/helpers.ts
(1 hunks)
🔇 Additional comments (2)
packages/ui/src/components/Pagination/helpers.ts (2)
5-5
: Doc fix aligns return semantics with implementation.The updated @returns text correctly reflects the inclusive behavior and the new guard.
10-10
: Good example for the equal-bounds case.This clarifies the intended single-page behavior.
if (start > end) { | ||
return []; | ||
} | ||
|
||
return [...Array(end - start + 1).keys()].map((key: number): number => key + start); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Inclusive boundary fix is correct; add a focused test and scan for callers relying on old behavior.
The start > end
guard is the right fix and keeps end - start + 1
non-negative. Please:
- Add a unit test specifically for
range(1, 1) -> [1]
and re-assertrange(5, 1) -> []
. - Verify no call sites depended on
start === end
returning[]
.
Run the following from repo root to review imports and usages:
Suggested tests (append to helpers.test.ts):
it('returns a single element when start equals end', () => {
expect(range(1, 1)).toEqual([1]);
});
it('returns empty array when start is greater than end', () => {
expect(range(5, 1)).toEqual([]);
});
🏁 Script executed:
#!/bin/bash
# Find modules importing Pagination helpers.range
rg -n -C2 -g '!**/node_modules/**' -g '!**/dist/**' -P 'import\s+\{\s*range\s*\}.*Pagination/helpers' packages
# Inspect all local usages within Pagination for possible assumptions on [] when start===end
rg -n -C3 -g '!**/node_modules/**' -g '!**/dist/**' -P '\brange\s*\(' packages/ui/src/components/Pagination
Length of output: 3620
Update tests for new inclusive-boundary behavior
The change to always include start === end
is correct, but the existing tests still expect range(10, 10)
to be []
. We need to:
• Adjust the first test in helpers.test.ts
so it only covers the start > end
case.
• Add a new test for the start === end
case to assert a single-element array.
• Verify that no other modules import or rely on the old range(start, end)
behavior outside of Pagination (scan confirmed only internal usage in Pagination.tsx
).
Affected file: packages/ui/src/components/Pagination/helpers.test.ts
Suggested diff:
describe("Helpers / Range", () => {
- it("should return the empty list, given start >= end", () => {
- expect(range(20, 10)).toEqual([]);
- expect(range(10, 10)).toEqual([]);
- });
+ it("should return the empty list, given start > end", () => {
+ expect(range(20, 10)).toEqual([]);
+ });
+ it("should return a single element, given start === end", () => {
+ expect(range(10, 10)).toEqual([10]);
+ });
it("should return every number from start to end, inclusive, given start < end", () => {
expect(range(10, 20)).toEqual([10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20]);
});
No external imports of range
were found beyond the Pagination component itself, so no other callers need changes.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
if (start > end) { | |
return []; | |
} | |
return [...Array(end - start + 1).keys()].map((key: number): number => key + start); | |
// packages/ui/src/components/Pagination/helpers.test.ts | |
describe("Helpers / Range", () => { | |
it("should return the empty list, given start > end", () => { | |
expect(range(20, 10)).toEqual([]); | |
}); | |
it("should return a single element, given start === end", () => { | |
expect(range(10, 10)).toEqual([10]); | |
}); | |
it("should return every number from start to end, inclusive, given start < end", () => { | |
expect(range(10, 20)).toEqual([ | |
10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20 | |
]); | |
}); | |
}); |
🤖 Prompt for AI Agents
In packages/ui/src/components/Pagination/helpers.test.ts around lines 14 to 17,
update the tests to reflect the inclusive-boundary change: modify the existing
first test so it only asserts the start > end case (e.g., range(11,10) yields
[]), and add a new test that asserts the start === end case returns a
single-element array (e.g., range(10,10) yields [10]); ensure test descriptions
reflect the cases and run the test suite to confirm no other failures — note
that a scan found no external imports beyond Pagination.tsx so no other modules
need changes.
Description
This PR fixes the pagination component not displaying page numbers when there's only one page (resolves #1048).
Changes Made
packages/ui/src/components/Pagination/helpers.ts
:start >= end
tostart > end
in therange
functionrange(1, 1)
returns[1]
Problem Solved
Previously, when the pagination component had only one page, the page number "1" would not be displayed, showing only disabled previous/next buttons. This was caused by the
range
function returning an empty array whenstart === end
.With this change:
range(1, 1)
now returns[1]
instead of[]
Related Issues
Testing
The existing test suite in
helpers.test.ts
already includes test cases that validate this behavior, includingrange(8, 9).toEqual([8, 9])
which confirms single-page ranges work correctly.Checklist
Changed condition from start >= end to start > end to allow range function to return [start] when start equals end. This fixes the pagination component not showing page 1 when there's only one page. Addresses issue #1048.
Summarize the changes made and the motivation behind them.
Reference related issues using
#
followed by the issue number.If there are breaking API changes - like adding or removing props, or changing the structure of the theme - describe them, and provide steps to update existing code.
Summary by CodeRabbit
Bug Fixes
Documentation