Skip to content

Conversation

rhamilto
Copy link
Member

@rhamilto rhamilto commented Sep 5, 2025

Pagination and filtering

Known upstream issue: Pagination controls are not available at smallest viewport sizes

Screen.Recording.2025-09-05.at.2.33.04.PM.mov

Sorting

Screen.Recording.2025-09-05.at.2.33.54.PM.mov

Different viewport sizes

Note the title layout issue is an existing bug.

Screen.Recording.2025-09-05.at.2.36.30.PM.mov

@openshift-ci-robot openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Sep 5, 2025
@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Sep 5, 2025
@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented Sep 5, 2025

@rhamilto: This pull request references CONSOLE-4760 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.21.0" version, but no target version was set.

In response to this:

Includes changes from #15375, which should merge first

Pagination and filtering

Screen.Recording.2025-09-05.at.2.33.04.PM.mov

Sorting

Screen.Recording.2025-09-05.at.2.33.54.PM.mov

Different viewport sizes (note the title layout issue is an existing bug)

Screen.Recording.2025-09-05.at.2.36.30.PM.mov

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 openshift-ci bot added component/core Related to console core functionality component/sdk Related to console-plugin-sdk approved Indicates a PR has been approved by an approver from all required OWNERS files. kind/i18n Indicates issue or PR relates to internationalization or has content that needs to be translated plugin-api-changed Categorizes a PR as containing plugin API changes labels Sep 5, 2025
@openshift-ci openshift-ci bot added the kind/cypress Related to Cypress e2e integration testing label Sep 8, 2025
@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented Sep 8, 2025

@rhamilto: This pull request references CONSOLE-4760 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.21.0" version, but no target version was set.

In response to this:

Includes changes from #15375, which should merge first

Pagination and filtering

Known upstream issue: patternfly/react-data-view#504

Screen.Recording.2025-09-05.at.2.33.04.PM.mov

Sorting

Screen.Recording.2025-09-05.at.2.33.54.PM.mov

Different viewport sizes (note the title layout issue is an existing bug)

Screen.Recording.2025-09-05.at.2.36.30.PM.mov

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
Copy link
Contributor

openshift-ci-robot commented Sep 8, 2025

@rhamilto: This pull request references CONSOLE-4760 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.21.0" version, but no target version was set.

In response to this:

Includes changes from #15375, which should merge first

Pagination and filtering

Known upstream issue: patternfly/react-data-view#504

Screen.Recording.2025-09-05.at.2.33.04.PM.mov

Sorting

Screen.Recording.2025-09-05.at.2.33.54.PM.mov

Different viewport sizes (note the title layout issue is an existing bug)

Screen.Recording.2025-09-05.at.2.36.30.PM.mov

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
Copy link
Contributor

openshift-ci-robot commented Sep 8, 2025

@rhamilto: This pull request references CONSOLE-4760 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.21.0" version, but no target version was set.

In response to this:

Includes changes from #15375, which should merge first

Pagination and filtering

Known upstream issue: patternfly/react-data-view#504

Screen.Recording.2025-09-05.at.2.33.04.PM.mov

Sorting

Screen.Recording.2025-09-05.at.2.33.54.PM.mov

Different viewport sizes

Note the title layout issue is an existing bug.

Screen.Recording.2025-09-05.at.2.36.30.PM.mov

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
Copy link
Member Author

rhamilto commented Sep 8, 2025

/retest

1 similar comment
@rhamilto
Copy link
Member Author

rhamilto commented Sep 8, 2025

/retest

@rhamilto rhamilto force-pushed the CONSOLE-4760 branch 3 times, most recently from 0552d27 to 8d4dc19 Compare September 8, 2025 21:30
@rhamilto
Copy link
Member Author

rhamilto commented Sep 9, 2025

code approval:
/assign @jhadvig
qe approval:
/assign @yapei
px approval:
/assign @sferich888
docs approval:
/assign @jseseCCS

Copy link
Contributor

openshift-ci bot commented Sep 9, 2025

@rhamilto: GitHub didn't allow me to assign the following users: jseseCCS.

Note that only openshift members with read permissions, repo collaborators and people who have commented on this issue/PR can be assigned. Additionally, issues/PRs can only have 10 assignees at the same time.
For more information please see the contributor guide

In response to this:

code approval:
/assign @jhadvig
qe approval:
/assign @yapei
px approval:
/assign @sferich888
docs approval:
/assign @jseseCCS

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.

@rhamilto rhamilto force-pushed the CONSOLE-4760 branch 2 times, most recently from c025b12 to f116745 Compare September 10, 2025 00:11
@yapei
Copy link
Contributor

yapei commented Sep 10, 2025

  • It looks like there is no Pagination in mobile view
no-pagination-on-mobile
  • filtering results seems not completely displayed on mobile
filter-on-mobile filter-on-browser

Other than above issues on mobile, the pagination, filtering, ordering works well on different browsers

@rhamilto
Copy link
Member Author

rhamilto commented Sep 10, 2025

  • It looks like there is no Pagination in mobile view

Good catch. This is an upstream bug.

  • filtering results seems not completely displayed on mobile

Hmm. It appears to be working for me. Can you elaborate on what you're seeing?

Screen.Recording.2025-09-10.at.9.32.37.AM.mov

return (
<div>
{!_.isEmpty(data) && Filter}
{!omitFilterToolbar && !_.isEmpty(data) && Filter}
Copy link
Member Author

Choose a reason for hiding this comment

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

Need to be able to omit the legacy <FilterToolbar> on list pages that are updated to use <ResourceDataView> as it includes its own filters.

@rhamilto
Copy link
Member Author

/retest

Copy link
Member

@TheRealJon TheRealJon left a comment

Choose a reason for hiding this comment

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

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Sep 12, 2025
Copy link
Contributor

openshift-ci bot commented Sep 12, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: rhamilto, TheRealJon

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Copy link
Member

@logonoff logonoff left a comment

Choose a reason for hiding this comment

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

aside from the known issues https://github.com/patternfly/react-data-view/issues/506, this looks great!

also: i am seeing a horizontal scrollbar in the empty state, but this only happens on Firefox. weird:

image

also also: maybe there should be some options to align the content within cells? I understand why we are aligning things to the top-left, but the alignment here looks a little off:

image

return (
<React.Suspense fallback={<LoadingBox />}>
<Table
<ResourceDataView
Copy link
Member

Choose a reason for hiding this comment

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

nit: this isn't really needed but I think adding type parameters is the intention when using this component

Suggested change
<ResourceDataView
<ResourceDataView<CustomResourceDefinitionKind>

Copy link
Member Author

Choose a reason for hiding this comment

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

@logonoff
Copy link
Member

/verified by @yapei

@openshift-ci-robot openshift-ci-robot added the verified Signifies that the PR passed pre-merge verification criteria label Sep 12, 2025
@openshift-ci-robot
Copy link
Contributor

@logonoff: This PR has been marked as verified by @yapei.

In response to this:

/verified by @yapei

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
Copy link
Member Author

/retest

@sferich888
Copy link

/label px-approved

@openshift-ci openshift-ci bot added the px-approved Signifies that Product Support has signed off on this PR label Sep 15, 2025
@jseseCCS
Copy link

Jocelyn here from docs. I approve this PR from a style/in-app help perspective. My only feedback is--and I realize this is unrelated to any changes in this PR--the Filter by Name field label is in headline style caps but should be sentence style: Filter by name. Please lmk if there's a more formal way I should give this feedback. Thanks! attn @rhamilto @sferich888

@rhamilto
Copy link
Member Author

rhamilto commented Sep 16, 2025

Jocelyn here from docs. I approve this PR from a style/in-app help perspective. My only feedback is--and I realize this is unrelated to any changes in this PR--the Filter by Name field label is in headline style caps but should be sentence style: Filter by name. Please lmk if there's a more formal way I should give this feedback. Thanks! attn @rhamilto @sferich888

@jseseCCS, great catch! If you're ok with it, I can address in a subsequent PR for the epic.

@rhamilto
Copy link
Member Author

Jocelyn here from docs. I approve this PR from a style/in-app help perspective. My only feedback is--and I realize this is unrelated to any changes in this PR--the Filter by Name field label is in headline style caps but should be sentence style: Filter by name. Please lmk if there's a more formal way I should give this feedback. Thanks! attn @rhamilto @sferich888

@jseseCCS, great catch! If you're ok with it, I can address in a subsequent PR for the epic.

Fix is in cb293cb#diff-ec7af5e524f203c58f7d45a7ebdb371c229f424ce0a1eccdccb7656bfec78b3c

@jseseCCS
Copy link

/label docs-approved

@openshift-ci openshift-ci bot added the docs-approved Signifies that Docs has signed off on this PR label Sep 18, 2025
@openshift-ci-robot
Copy link
Contributor

/retest-required

Remaining retests: 0 against base HEAD df5f06d and 2 for PR HEAD 1c87ccc in total

@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented Sep 18, 2025

@rhamilto: This pull request references CONSOLE-4760 which is a valid jira issue.

In response to this:

Pagination and filtering

Known upstream issue: Pagination controls are not available at smallest viewport sizes

Screen.Recording.2025-09-05.at.2.33.04.PM.mov

Sorting

Screen.Recording.2025-09-05.at.2.33.54.PM.mov

Different viewport sizes

Note the title layout issue is an existing bug.

Screen.Recording.2025-09-05.at.2.36.30.PM.mov

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
Copy link
Member Author

/retest

@openshift-ci-robot
Copy link
Contributor

/retest-required

Remaining retests: 0 against base HEAD 6098ca4 and 2 for PR HEAD 1c87ccc in total

@openshift-ci-robot
Copy link
Contributor

/retest-required

Remaining retests: 0 against base HEAD 6efa508 and 2 for PR HEAD 1c87ccc in total

Copy link
Contributor

openshift-ci bot commented Sep 19, 2025

@rhamilto: all tests passed!

Full PR test history. Your PR dashboard.

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. I understand the commands that are listed here.

@openshift-merge-bot openshift-merge-bot bot merged commit 68f6c11 into openshift:main Sep 19, 2025
8 checks passed
@logonoff logonoff deleted the CONSOLE-4760 branch September 19, 2025 11:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. component/core Related to console core functionality component/sdk Related to console-plugin-sdk docs-approved Signifies that Docs has signed off on this PR jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. kind/cypress Related to Cypress e2e integration testing kind/i18n Indicates issue or PR relates to internationalization or has content that needs to be translated lgtm Indicates that a PR is ready to be merged. plugin-api-approved Indicates a PR with plugin API changes has been approved by an API reviewer plugin-api-changed Categorizes a PR as containing plugin API changes px-approved Signifies that Product Support has signed off on this PR qe-approved Signifies that QE has signed off on this PR verified Signifies that the PR passed pre-merge verification criteria
Projects
None yet
Development

Successfully merging this pull request may close these issues.