2525 Allow IcPaginationBar items per page to be set programmatically #2999

dn55533 · 2025-01-10T10:40:17Z

Summary of the changes

A new selectedItemsPerPage prop has been added to the IcPaginationBar that allows the selected items per page dropdown option to be set programmatically.

Error handling has been included such that if the selectedItemsPerPage prop is set to a value that does not correspond to one of the available items per page dropdown options, an error will appear in the console.

Also, some error handling has been added to the IcPaginationBar currentPage prop, because it was possible to set the current page to a number that was larger than the total number of pages. Now, if this is attempted the modification of the current page is prevented and an error appears in the console.

Also, a very minor code tidy-up has been performed in the IcDataTable.

Related issue

#2525

Checklist

General

Changes to docs package checked and committed.
All acceptance criteria reviewed and met.

Testing

Relevant unit tests and visual regression tests added.
Visual testing against Figma component specification completed.
Playground stories in React Storybook up to date, with any prop changes and additions addressed.
Compare performance of modified components against develop using Performance addon in React Storybook.

Accessibility

Accessibility Insights FastPass performed.
A11y unit test added and yields no issues.
A11y plug-in on Storybook yields no issues.
Manual screen reader testing performed using NVDA and VoiceOver.
Manual keyboard testing for keyboard controls and logical focus order.
Correct roles used and ARIA attributes used correctly where required.
Logical heading structure is maintained, and the HTML elements used for headings can be changed to fit within the wider page structure.

Resize/zoom behaviour

Page can be zoomed to 400% with no loss of content.
Screen magnifier used with no issues.
Text resized to 200% with no loss of content.
Text spacing increased as per the WCAG 1.4.12 success criterion with no loss of content.

System modes

Browser setting 'prefers reduced motion' tested. No animations or motion visible whilst this setting is on.
Windows High Contrast mode tested with no loss of content.
System light and dark mode tested with no loss of content.
Browser support tested (Chrome, Safari, Firefox and Edge).

Testing content extremes

Min/max content examples tested with no loss of content or overflow.
All prop combinations work without issue.
Tested for FOUC (Flash of Unstyled Content) in both SSR (Server-Side Rendering) and SSG (Static Site Generation) settings.
Controlled and uncontrolled input components tested.
Props/slots can be updated after initial render.

packages/canary-react/src/stories/ic-pagination-bar.stories.mdx

ad9242 · 2025-01-16T13:06:20Z

we seem to be seeing the same issue here with generated images. if you want to add a .skip for these 2 & we will get them added back in after this is merged, please feel free

GCHQ-Developer-847

Just spotted a couple of things

Hopefully a rebase should fix the audit issue as well :)

GCHQ-Developer-847 · 2025-01-29T17:27:51Z

packages/canary-web-components/src/components/ic-pagination-bar/ic-pagination-bar.tsx

@@ -85,7 +85,40 @@ export class PaginationBar {

  @Watch("currentPage")
  watchPageNumberHandler(): void {
-    this.activePage = this.currentPage;
+    if (typeof this.currentPage === "number" && this.currentPage) {
+      if (this.totalPages && this.currentPage > this.totalPages) {


Should this if statement also check that currentPage is greater than zero?

Suggested change

if (this.totalPages && this.currentPage > this.totalPages) {

if (this.totalPages && this.currentPage > this.totalPages || this.currentPage < 1) {

Agreed - I have added this check and also added a new story to ic-pagination-bar that demonstrates the validation in action.

I have also added a new story to ic-pagination-bar to show the selectedItemsPerPage prop when this is set to an invalid value.

GCHQ-Developer-847 · 2025-01-29T17:38:31Z

packages/canary-web-components/src/components/ic-pagination-bar/ic-pagination-bar.tsx

+          (option) => option.value === `${this.selectedItemsPerPage}`
+        ).length
+      ) {
+        this.itemsPerPageString = `${this.selectedItemsPerPage}`;


Is this line needed? this.itemsPerPageString gets updated within the setItemsPerPage function which is being called below

Yes well spotted - I have removed this.

GCHQ-Developer-847

See comments above

ad9242

Building locally (npm build:all) is showing some changes to the docs & date-input readme. Would you mind just checking if it's showing any uncommitted changes for you?

ad9242 · 2025-02-04T09:04:45Z

packages/canary-web-components/src/components/ic-pagination-bar/ic-pagination-bar.tsx

+        (this.totalPages && this.currentPage > this.totalPages)
+      ) {
+        console.error(
+          "Current page must be a number greater than zero but less than or equal to the total number of pages"


might be nice to show the value it's been given, like you have for the selectedItemsPerPage console error?

Agreed - I have updated this so it now includes the current page and the total number of pages in the console error message. At the same time I also fixed a bug in the validation of current page > total pages.

ad9242 · 2025-02-04T14:12:58Z

An extension to this could be the ability to set the selected option to "All", but happy for this to be a separate ticket

dn55533 · 2025-02-04T17:29:51Z

An extension to this could be the ability to set the selected option to "All", but happy for this to be a separate ticket

The selected items per page option is passed in using its numerical value. So it the available dropdown options are: [10, 25, All] and there are, say, 120 items, if you pass in 120 then the All option will become selected.

Perhaps there is a clearer way of doing this, if so then yes it would be better in a separate ticket.

dn55533 · 2025-02-04T19:50:50Z

Building locally (npm build:all) is showing some changes to the docs & date-input readme. Would you mind just checking if it's showing any uncommitted changes for you?

There were 3 files with some minor content changes, I have added these to the PR

ad9242

Please can you remove the docs scope commit? It shouldn't be introduced by your changes, so we will fix separately rather than make it part of this work

…rammatically Provide a selectedItemsPerPage prop fix mi6#2525

…e set programmatically Provide a selectedItemsPerPage prop fix mi6#2525

…grammatically Provide a selectedItemsPerPage prop fix mi6#2525

dn55533 marked this pull request as draft January 10, 2025 11:14

dn55533 force-pushed the fix/2525-set-icpaginationbar-items-per-page-programmatically branch from d28efd8 to 9b607d5 Compare January 10, 2025 12:01

dn55533 marked this pull request as ready for review January 10, 2025 12:03

dn55533 force-pushed the fix/2525-set-icpaginationbar-items-per-page-programmatically branch from 9b607d5 to d0128b9 Compare January 10, 2025 13:50

ad9242 requested changes Jan 13, 2025

View reviewed changes

packages/canary-react/src/stories/ic-pagination-bar.stories.mdx Outdated Show resolved Hide resolved

dn55533 force-pushed the fix/2525-set-icpaginationbar-items-per-page-programmatically branch 2 times, most recently from e3ac8c3 to 3a6ea44 Compare January 15, 2025 16:05

dn55533 force-pushed the fix/2525-set-icpaginationbar-items-per-page-programmatically branch 4 times, most recently from 461f7f3 to 0a22433 Compare January 20, 2025 17:19

GCHQ-Developer-741 previously approved these changes Jan 21, 2025

View reviewed changes

dn55533 dismissed GCHQ-Developer-741’s stale review via bbfcfe5 January 22, 2025 16:04

dn55533 force-pushed the fix/2525-set-icpaginationbar-items-per-page-programmatically branch from 0a22433 to bbfcfe5 Compare January 22, 2025 16:04

GCHQ-Developer-741 previously approved these changes Jan 23, 2025

View reviewed changes

GCHQ-Developer-847 reviewed Jan 29, 2025

View reviewed changes

GCHQ-Developer-847 requested changes Jan 29, 2025

View reviewed changes

dn55533 force-pushed the fix/2525-set-icpaginationbar-items-per-page-programmatically branch 2 times, most recently from 435640a to dbc5977 Compare January 30, 2025 17:28

dn55533 dismissed GCHQ-Developer-741’s stale review via 90b0b7f January 31, 2025 11:09

dn55533 force-pushed the fix/2525-set-icpaginationbar-items-per-page-programmatically branch from dbc5977 to 90b0b7f Compare January 31, 2025 11:09

ad9242 requested changes Feb 4, 2025

View reviewed changes

ad9242 reviewed Feb 4, 2025

View reviewed changes

dn55533 force-pushed the fix/2525-set-icpaginationbar-items-per-page-programmatically branch from 90b0b7f to 9e7105e Compare February 4, 2025 09:23

dn55533 force-pushed the fix/2525-set-icpaginationbar-items-per-page-programmatically branch from 9e7105e to 13e94bf Compare February 4, 2025 19:44

ad9242 requested changes Feb 6, 2025

View reviewed changes

dn55533 force-pushed the fix/2525-set-icpaginationbar-items-per-page-programmatically branch from 13e94bf to 31c1ab7 Compare February 6, 2025 13:46

dn55533 added 3 commits February 6, 2025 14:08

fix(canary-docs): allow IcPaginationBar items per page to be set prog…

67b839d

…rammatically Provide a selectedItemsPerPage prop fix mi6#2525

fix(canary-web-components): allow IcPaginationBar items per page to b…

9e297e7

…e set programmatically Provide a selectedItemsPerPage prop fix mi6#2525

fix(canary-react): allow IcPaginationBar items per page to be set pro…

1bc01f8

…grammatically Provide a selectedItemsPerPage prop fix mi6#2525

dn55533 force-pushed the fix/2525-set-icpaginationbar-items-per-page-programmatically branch from 31c1ab7 to 1bc01f8 Compare February 6, 2025 14:12

ad9242 approved these changes Feb 10, 2025

View reviewed changes

GCHQ-Developer-299 approved these changes Feb 11, 2025

View reviewed changes

GCHQ-Developer-847 approved these changes Feb 11, 2025

View reviewed changes

GCHQ-Developer-847 merged commit 146c81a into mi6:develop Feb 11, 2025
6 checks passed

This was referenced Feb 11, 2025

Enables skipped pagination bar tests #3170

Merged

V3 cherry pick set pagination programmatically #3184

Merged

dn55533 deleted the fix/2525-set-icpaginationbar-items-per-page-programmatically branch February 14, 2025 10:25

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

2525 Allow IcPaginationBar items per page to be set programmatically #2999

2525 Allow IcPaginationBar items per page to be set programmatically #2999

dn55533 commented Jan 10, 2025 •

edited

Loading

ad9242 commented Jan 16, 2025

GCHQ-Developer-847 left a comment

GCHQ-Developer-847 Jan 29, 2025

dn55533 Jan 31, 2025

GCHQ-Developer-847 Jan 29, 2025

dn55533 Jan 31, 2025

GCHQ-Developer-847 left a comment

ad9242 left a comment

ad9242 Feb 4, 2025

dn55533 Feb 4, 2025 •

edited

Loading

ad9242 commented Feb 4, 2025

dn55533 commented Feb 4, 2025

dn55533 commented Feb 4, 2025

ad9242 left a comment

	if (this.totalPages && this.currentPage > this.totalPages) {
	if (this.totalPages && this.currentPage > this.totalPages \|\| this.currentPage < 1) {

2525 Allow IcPaginationBar items per page to be set programmatically #2999

2525 Allow IcPaginationBar items per page to be set programmatically #2999

Conversation

dn55533 commented Jan 10, 2025 • edited Loading

Summary of the changes

Related issue

Checklist

General

Testing

Accessibility

Resize/zoom behaviour

System modes

Testing content extremes

ad9242 commented Jan 16, 2025

GCHQ-Developer-847 left a comment

Choose a reason for hiding this comment

GCHQ-Developer-847 Jan 29, 2025

Choose a reason for hiding this comment

dn55533 Jan 31, 2025

Choose a reason for hiding this comment

GCHQ-Developer-847 Jan 29, 2025

Choose a reason for hiding this comment

dn55533 Jan 31, 2025

Choose a reason for hiding this comment

GCHQ-Developer-847 left a comment

Choose a reason for hiding this comment

ad9242 left a comment

Choose a reason for hiding this comment

ad9242 Feb 4, 2025

Choose a reason for hiding this comment

dn55533 Feb 4, 2025 • edited Loading

Choose a reason for hiding this comment

ad9242 commented Feb 4, 2025

dn55533 commented Feb 4, 2025

dn55533 commented Feb 4, 2025

ad9242 left a comment

Choose a reason for hiding this comment

dn55533 commented Jan 10, 2025 •

edited

Loading

dn55533 Feb 4, 2025 •

edited

Loading