Skip to content

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix(pagination): allow single page display by updating range condition #1618

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions packages/ui/src/components/Pagination/helpers.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,17 +2,17 @@
* Generates an array of sequential numbers from a start value to an end value (inclusive).
* @param start - The starting number of the range
* @param end - The ending number of the range
* @returns An array of numbers from start to end. Returns empty array if start is greater than or equal to end.
* @returns An array of numbers from start to end. Returns empty array if start is greater than end.
* @example
* ```ts
* range(1, 5) // returns [1, 2, 3, 4, 5]
* range(5, 1) // returns []
* range(1, 1) // returns [1]
* ```
*/
export function range(start: number, end: number): number[] {
if (start >= end) {
if (start > end) {
return [];
}

return [...Array(end - start + 1).keys()].map((key: number): number => key + start);
Comment on lines +14 to 17
Copy link
Contributor

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-assert range(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.

Suggested change
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.

}
Loading