Skip to content

Conversation

@rhamilto
Copy link
Member

@rhamilto rhamilto commented Jul 9, 2025

…Display

@openshift-ci-robot openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Jul 9, 2025
@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented Jul 9, 2025

@rhamilto: This pull request references CONSOLE-4659 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.20.0" version, but no target version was set.

Details

In response to this:

…Display

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 requested review from cajieh and sg00dwin July 9, 2025 14:04
@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. component/shared Related to console-shared labels Jul 9, 2025
@rhamilto
Copy link
Member Author

rhamilto commented Jul 9, 2025

code approval:
/assign @logonoff
qe approval:
/assign @yapei

This is tech debt, so nothing for PX or docs to review.
/label px-approved
/label docs-approved

@openshift-ci openshift-ci bot added px-approved Signifies that Product Support has signed off on this PR docs-approved Signifies that Docs has signed off on this PR labels Jul 9, 2025
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.

/lgtm

🥳

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jul 9, 2025
@sg00dwin
Copy link
Member

sg00dwin commented Jul 9, 2025

Robb, These breakpoint choices may have been intentional, I just wanted to note that the bootstrap responsive class size names (-xs, -sm, etc) do not correspond to the size names for the Patternfly global breakpoints.

So a one for one change that would maintain consistency for...
hidden-xs corresponds to pf-v6-u-display-none pf-v6-u-display-*-on-md and
hidden-xs hidden-sm corresponds to pf-v6-u-display-none pf-v6-u-display-*-on-lg

@rhamilto
Copy link
Member Author

rhamilto commented Jul 9, 2025

Robb, These breakpoint choices may have been intentional, I just wanted to note that the bootstrap responsive class size names (-xs, -sm, etc) do not correspond to the size names for the Patternfly global breakpoints.

So a one for one change that would maintain consistency for... hidden-xs corresponds to pf-v6-u-display-none pf-v6-u-display-*-on-md and hidden-xs hidden-sm corresponds to pf-v6-u-display-none pf-v6-u-display-*-on-lg

Thanks, @sg00dwin! In my testing, the changes I've included provide the intended functionality even though they aren't an exact match.

@rhamilto
Copy link
Member Author

rhamilto commented Jul 9, 2025

/test okd-scos-e2e-aws-ovn

3 similar comments
@rhamilto
Copy link
Member Author

rhamilto commented Jul 9, 2025

/test okd-scos-e2e-aws-ovn

@rhamilto
Copy link
Member Author

/test okd-scos-e2e-aws-ovn

@rhamilto
Copy link
Member Author

/test okd-scos-e2e-aws-ovn

return (
<span className="co-resource-item">
<span className="co-resource-icon--fixed-width hidden-xs">
<span className="co-resource-icon--fixed-width pf-v6-u-display-none pf-v6-u-display-flex-on-sm">
Copy link
Contributor

Choose a reason for hiding this comment

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

resource icon on API Explorer list page is still visible on smallest width
Screenshot 2025-07-11 at 11 45 12 AM

Comment on lines 25 to 29
'',
'',
'',
'pf-v6-u-display-none pf-v6-u-display-table-cell-on-sm',
Kebab.columnClass,
Copy link
Contributor

Choose a reason for hiding this comment

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

all default columns are still visible at smallest width
Screenshot 2025-07-11 at 11 52 05 AM

Copy link
Member Author

@rhamilto rhamilto Jul 11, 2025

Choose a reason for hiding this comment

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

Updated.

Screen.Recording.2025-07-11.at.8.25.02.AM.mov

type="submit"
variant="secondary"
className="pf-v6-c-button--align-right hidden-sm hidden-xs"
className="pf-v6-c-button--align-right pf-v6-u-display-none pf-v6-u-display-flex-on-sm"
Copy link
Contributor

Choose a reason for hiding this comment

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

I can still see Download button at smallest screen width
Screenshot 2025-07-11 at 11 58 29 AM

{obj.namespace && (
<ResourceLink
className="co-sysevent__resourcelink hidden-xs"
className="co-sysevent__resourcelink pf-v6-u-display-none pf-v6-u-display-block-on-sm"
Copy link
Contributor

Choose a reason for hiding this comment

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

Namespace link still visible at smallest screen width
Screenshot 2025-07-11 at 12 00 51 PM

return (
<div
className="co-p-has-sidebar__sidebar co-p-has-sidebar__sidebar--bordered hidden-sm hidden-xs"
className="co-p-has-sidebar__sidebar co-p-has-sidebar__sidebar--bordered pf-v6-u-display-none pf-v6-u-display-block-on-sm"
Copy link
Contributor

Choose a reason for hiding this comment

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

this seems working well

Screen.Recording.2025-07-11.at.12.08.30.PM.mov

</span>
{resourceStatus && (
<ResourceStatus additionalClassNames="hidden-xs">
<ResourceStatus additionalClassNames="pf-v6-u-display-none pf-v6-u-display-block-on-sm">
Copy link
Contributor

Choose a reason for hiding this comment

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

this seems working fine too
Screenshot 2025-07-11 at 12 12 31 PM

@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Jul 11, 2025
@rhamilto
Copy link
Member Author

Excellent reviewing, @yapei. As mentioned in #15261 (comment), I utilized slightly different breakpoints as we have the ability to maintain more of the elements as there is sufficient space to display them (new breakpoint is 576px).

Screen.Recording.2025-07-11.at.8.26.17.AM.mov
Screen.Recording.2025-07-11.at.8.25.50.AM.mov
Screen.Recording.2025-07-11.at.8.22.03.AM.mov

The one exception you noted is the ConfigMaps list page, which I've updated. Good catch.

Screen.Recording.2025-07-11.at.8.25.02.AM.mov

@rhamilto
Copy link
Member Author

/retest

@yapei
Copy link
Contributor

yapei commented Jul 14, 2025

Thanks for the quick updates!! tested again and working fine
Screenshot 2025-07-14 at 3 21 50 PM

Screenshot 2025-07-14 at 3 24 06 PM

/label qe-approved

@openshift-ci openshift-ci bot added the qe-approved Signifies that QE has signed off on this PR label Jul 14, 2025
@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented Jul 14, 2025

@rhamilto: This pull request references CONSOLE-4659 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.20.0" version, but no target version was set.

Details

In response to this:

…Display

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

/test okd-scos-e2e-aws-ovn

1 similar comment
@rhamilto
Copy link
Member Author

/test okd-scos-e2e-aws-ovn

@rhamilto
Copy link
Member Author

/retest

1 similar comment
@rhamilto
Copy link
Member Author

/retest

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.

/lgtm

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

openshift-ci bot commented Jul 15, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: logonoff, rhamilto, sg00dwin

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

The pull request process is described here

Details 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

@rhamilto
Copy link
Member Author

/retest

@openshift-merge-bot openshift-merge-bot bot merged commit 1628346 into openshift:main Jul 15, 2025
6 of 7 checks passed
@rhamilto rhamilto deleted the CONSOLE-4659 branch July 15, 2025 21:17
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 component/shared Related to console-shared 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. lgtm Indicates that a PR is ready to be merged. px-approved Signifies that Product Support has signed off on this PR qe-approved Signifies that QE has signed off on this PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants