OCPBUGS-84876: Add bottom pagination to ConsoleDataView for mobile responsiveness#16391
Conversation
openshift-ci-robot
added
jira/severity-moderate
|
@rhamilto: GitHub didn't allow me to request PR reviews from the following users: openshift/team-ux-review. Note that only openshift members and repo collaborators can review this PR, and authors cannot review their own PRs. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
openshift-ci-robot
added
the
jira/invalid-bug
|
@rhamilto: This pull request references Jira Issue OCPBUGS-84876, which is invalid:
Comment The bug has been updated to refer to the pull request using the external bug tracker. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
openshift-ci
Bot
added
component/core
|
/cherry-pick release-4.22 release-4.21 |
|
@rhamilto: once the present PR merges, I will cherry-pick it on top of DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
| <Pagination | ||
| itemCount={filteredData.length} | ||
| titles={{ | ||
| ofWord: t('public~of'), | ||
| itemsPerPage: t('public~Items per page'), | ||
| perPageSuffix: t('public~per page'), | ||
| }} | ||
| variant={PaginationVariant.bottom} | ||
| {...pagination} | ||
| /> |
There was a problem hiding this comment.
According to the data view example, they wrap this in another DataViewToolbar:
<DataView selection={selection} activeState={finalData.length > 0 ? undefined : 'empty'}>
// ...
<DataViewToolbar
ouiaId='LayoutExampleFooter'
pagination={
<Pagination
isCompact
perPageOptions={perPageOptions}
itemCount={repositories.length}
{...pagination}
/>
}
/>
</DataView>
📝 WalkthroughWalkthroughThis change refactors the pagination UI in 🚥 Pre-merge checks | ✅ 12✅ Passed checks (12 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
frontend/packages/console-app/src/components/data-view/ConsoleDataView.tsx (1)
249-258: ⚡ Quick winDeduplicate shared pagination config to prevent drift.
The same
titlesanditemCountconfig is now duplicated between the top and bottom pagers. Extracting shared pagination props keeps both controls consistent as this evolves.♻️ Suggested refactor
+ const paginationTitles = useMemo( + () => ({ + ofWord: t('public~of'), + itemsPerPage: t('public~Items per page'), + perPageSuffix: t('public~per page'), + }), + [t], + ); ... pagination={ <Pagination itemCount={filteredData.length} - titles={{ - ofWord: t('public~of'), - itemsPerPage: t('public~Items per page'), - perPageSuffix: t('public~per page'), - }} + titles={paginationTitles} {...pagination} /> } ... <Pagination itemCount={filteredData.length} - titles={{ - ofWord: t('public~of'), - itemsPerPage: t('public~Items per page'), - perPageSuffix: t('public~per page'), - }} + titles={paginationTitles} variant={PaginationVariant.bottom} {...pagination} />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/packages/console-app/src/components/data-view/ConsoleDataView.tsx` around lines 249 - 258, Extract the duplicated pagination props into a shared object (e.g., const sharedPaginationProps = { itemCount: filteredData.length, titles: { ofWord: t('public~of'), itemsPerPage: t('public~Items per page'), perPageSuffix: t('public~per page') }, variant: PaginationVariant.bottom }) and spread that into the bottom <Pagination ... /> where Pagination, filteredData, pagination are used; then replace the duplicated itemCount/titles in the top pager to use the same sharedPaginationProps (spreading {...sharedPaginationProps, ...pagination} as appropriate) so both pagers consume one source of truth and avoid drift.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@frontend/packages/console-app/src/components/data-view/ConsoleDataView.tsx`:
- Around line 249-258: Extract the duplicated pagination props into a shared
object (e.g., const sharedPaginationProps = { itemCount: filteredData.length,
titles: { ofWord: t('public~of'), itemsPerPage: t('public~Items per page'),
perPageSuffix: t('public~per page') }, variant: PaginationVariant.bottom }) and
spread that into the bottom <Pagination ... /> where Pagination, filteredData,
pagination are used; then replace the duplicated itemCount/titles in the top
pager to use the same sharedPaginationProps (spreading
{...sharedPaginationProps, ...pagination} as appropriate) so both pagers consume
one source of truth and avoid drift.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: b036d057-291f-470a-8546-639f9179ad8f
📒 Files selected for processing (1)
frontend/packages/console-app/src/components/data-view/ConsoleDataView.tsx
📜 Review details
🔇 Additional comments (1)
frontend/packages/console-app/src/components/data-view/ConsoleDataView.tsx (1)
9-9: PatternFly import update looks correct.
PaginationVariantis imported from the correct module and matches the new bottom paginator usage.
openshift-ci
Bot
added
the
kind/i18n
rhamilto
force-pushed
the
OCPBUGS-84876
branch
2 times, most recently
from
7e34e35 to
7e5145c
Compare
…sponsiveness The ConsoleDataView component was missing bottom pagination, which is required by PatternFly design guidelines for proper mobile responsiveness. On mobile viewports, the top pagination condenses to just an item count, so full pagination controls must be available in the footer/bottom. This change adds a Pagination component with variant="bottom" after the DataViewTable to ensure pagination controls remain accessible on all viewport sizes. Additionally, adds complete i18n support to both top and bottom pagination components with proper aria-labels for navigation buttons (first, previous, next, last pages) and translated visible text (ofWord, itemsPerPage, perPageSuffix) to ensure full accessibility and internationalization. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: logonoff, rhamilto The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/verified by @rhamilto |
openshift-ci-robot
added
the
verified
|
@rhamilto: This PR has been marked as verified by DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
/jira refresh |
openshift-ci-robot
added
jira/valid-bug
|
@rhamilto: This pull request references Jira Issue OCPBUGS-84876, which is valid. The bug has been moved to the POST state. 3 validation(s) were run on this bug
DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
openshift-ci-robot
removed
the
verified
|
New changes are detected. LGTM label has been removed. |
|
/override ci/prow/e2e-gcp-console I mistakenly pushed a change to the branch. The tests previously passed. |
|
@rhamilto: Overrode contexts on behalf of rhamilto: ci/prow/e2e-gcp-console DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
/verified by @rhamilto |
openshift-ci-robot
added
the
verified
|
@rhamilto: This PR has been marked as verified by DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
@rhamilto: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
|
@rhamilto: Jira Issue Verification Checks: Jira Issue OCPBUGS-84876 Jira Issue OCPBUGS-84876 has been moved to the MODIFIED state and will move to the VERIFIED state when the change is available in an accepted nightly payload. 🕓 DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
@rhamilto: new pull request created: #16394 DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
4 participants
Analysis / Root cause:
The ConsoleDataView component only included pagination in the top toolbar. According to PatternFly design guidelines (https://www.patternfly.org/components/pagination/design-guidelines/), both top and bottom pagination should be used. On mobile viewports (< 768px), the top pagination condenses to just an item count, leaving users with no way to navigate between pages.
Additionally, the pagination components were missing internationalization (i18n) for navigation button aria-labels and visible text, which is required for accessibility and localization support.
Solution description:
Paginationcomponent withvariant={PaginationVariant.bottom}after theDataViewTablecomponenttoFirstPageAriaLabel,toPreviousPageAriaLabel,toNextPageAriaLabel,toLastPageAriaLabelofWord,itemsPerPage,perPageSuffixPaginationVariantimport from@patternfly/react-core{...pagination}) to keep them synchronizedScreenshots / screen recording:
Test setup:
No special setup required.
Test cases:
Browser conformance:
Additional info:
Reviewers and assignees:
/cc @openshift/team-ux-review