OCPBUGS-54670: Use PF component group for PageHeading #14965

logonoff · 2025-04-12T23:48:12Z

https://issues.redhat.com//browse/CONSOLE-4492
https://issues.redhat.com/browse/OCPBUGS-54670

follow up on #14875

adopts PF PageHeader for console's main heading component.

no major UI changes should appear in this PR (e.g., create buttons appearing in a separate row as the title), except for two: operator details now have a line between the operator icon and the operator name. badges are also now placed next to the title.

fixes small a11y issue where heading actions were inside of the h1 element when it shouldn't be

openshift-ci-robot · 2025-04-12T23:48:16Z

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

Details

In response to this:

/hold

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.

logonoff · 2025-04-13T11:54:11Z

/unhold

logonoff · 2025-04-13T18:05:34Z

qe review:
/assign @yapei

openshift-ci-robot · 2025-04-13T18:47:29Z

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

Details

In response to this:

https://issues.redhat.com//browse/CONSOLE-4492

follow up on #14875

adopts PF PageHeader for console's main heading component.

no major UI changes should appear in this PR (e.g., create buttons appearing in a separate row as the title), except for two: operator details now have a line between the operator icon and the operator name. badges are also now placed next to the title.

fixes small a11y issue where heading actions were inside of the h1 element when it shouldn't be

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.

yapei · 2025-04-14T09:32:04Z

Check Edit YAML link and style on StorageClass, VolumeSnapshot, PersistentVolumeClaim creation page
Check resource detail page when a badge is present, such as pod details page

- Check operator details page heading

- Also tested against Chrome, Safari, iPhone devices

No outstanding layout issues found
/label qe-approved

openshift-ci-robot · 2025-04-14T09:32:12Z

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

Details

In response to this:

https://issues.redhat.com//browse/CONSOLE-4492

follow up on #14875

adopts PF PageHeader for console's main heading component.

no major UI changes should appear in this PR (e.g., create buttons appearing in a separate row as the title), except for two: operator details now have a line between the operator icon and the operator name. badges are also now placed next to the title.

fixes small a11y issue where heading actions were inside of the h1 element when it shouldn't be

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.

logonoff · 2025-04-14T11:19:20Z

code review:
/assign @rhamilto

/label px-approved

Removed children

frontend/packages/console-shared/src/components/heading/PageHeading.scss

- adopt `BasePageHeading` when possible (as many pages don't use the extra functionality provided by the `connect`) - Rename `PageHeading` to `ConnectedPageHeading` - Rename `BasePageHeading` to `PageHeading` - move `PageHeading`, `Breadcrumbs` to console/shared - update cypress test to reflect changes

rhamilto

/lgtm

Let's see if we can't get this in before branching and address any issues in follow-ons.

rhamilto · 2025-04-23T15:18:47Z

frontend/packages/console-dynamic-plugin-sdk/src/extensions/console-types.ts

+   * is not the primary page header to avoid having multiple favourites buttons.
+   */
  hideFavoriteButton?: boolean;
+  /** The heading title. If no title is set, only the `children`, `badge`, and `helpAlert` props will be rendered */


Not new, but very confusing since title is required. Maybe this should read If title=''...?

The funny thing though is that we set title={undefined} all the time in our codebase. See the ListPage/FireMan components, where if showTitle is set to false, then it explicitly passes undefined to the title. It appears this has been the behaviour since at least 2017.

Even weirder.

frontend/packages/console-shared/src/components/heading/PageHeading.tsx

frontend/public/style/_vars.scss

openshift-ci · 2025-04-23T17:47:22Z

[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

Details

Needs approval from an approver in each of these files:

~~frontend/OWNERS~~ [logonoff,rhamilto]

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

logonoff · 2025-04-23T17:51:26Z

/retest

vojtechszocs · 2025-04-23T18:26:17Z

/label plugin-api-approved

openshift-ci-robot · 2025-04-23T18:33:01Z

/retest-required

Remaining retests: 0 against base HEAD bd6ef9a and 2 for PR HEAD a9afa41 in total

logonoff · 2025-04-24T02:09:49Z

/retest-required

openshift-ci-robot · 2025-04-24T05:26:49Z

/retest-required

Remaining retests: 0 against base HEAD bd6ef9a and 2 for PR HEAD a9afa41 in total

rhamilto · 2025-04-24T09:48:10Z

/retest

jhadvig · 2025-04-24T13:32:11Z

Overriding the CI job due to flakiness

/override ci/prow/e2e-gcp-console

openshift-ci · 2025-04-24T13:34:08Z

@jhadvig: Overrode contexts on behalf of jhadvig: ci/prow/e2e-gcp-console

Details

In response to this:

Overriding the CI job due to flakiness

/override ci/prow/e2e-gcp-console

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.

logonoff · 2025-04-24T13:34:59Z

/label docs-approved

openshift-ci · 2025-04-24T14:31:51Z

@logonoff: all tests passed!

Full PR test history. Your PR dashboard.

Details

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-ci-robot · 2025-04-24T14:35:42Z

@logonoff: Jira Issue OCPBUGS-54670: All pull requests linked via external trackers have merged:

openshift/console#14965

Jira Issue OCPBUGS-54670 has been moved to the MODIFIED state.

Details

In response to this:

https://issues.redhat.com//browse/CONSOLE-4492
https://issues.redhat.com/browse/OCPBUGS-54670

follow up on #14875

adopts PF PageHeader for console's main heading component.

no major UI changes should appear in this PR (e.g., create buttons appearing in a separate row as the title), except for two: operator details now have a line between the operator icon and the operator name. badges are also now placed next to the title.

fixes small a11y issue where heading actions were inside of the h1 element when it shouldn't be

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-merge-robot · 2025-05-02T10:36:09Z

Fix included in accepted release 4.19.0-0.nightly-2025-04-29-095709

openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Apr 12, 2025

openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Apr 12, 2025

openshift-ci bot requested review from cajieh and karthikjeeyar April 12, 2025 23:48

openshift-ci bot assigned jhadvig, spadgett and vojtechszocs Apr 12, 2025

logonoff force-pushed the pageheader branch from e2b2b12 to c7727f2 Compare April 13, 2025 04:06

openshift-ci bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Apr 13, 2025

logonoff changed the title ~~CONSOLE-4492: Use PF component group for PageHeader~~ CONSOLE-4492: Use PF component group for PageHeading Apr 13, 2025

openshift-ci bot assigned yapei Apr 13, 2025

logonoff force-pushed the pageheader branch from c7727f2 to dabff31 Compare April 13, 2025 20:12

openshift-ci bot added the kind/i18n Indicates issue or PR relates to internationalization or has content that needs to be translated label Apr 13, 2025

logonoff force-pushed the pageheader branch from dabff31 to 967bb49 Compare April 13, 2025 20:15

openshift-ci bot added the qe-approved Signifies that QE has signed off on this PR label Apr 14, 2025

logonoff and others added 4 commits April 22, 2025 23:53

CONSOLE-4492: Remove actions dropdown and breadcrumb extra spacing

d1bc886

CONSOLE-4492: remove NavTitle and PrimaryHeading

4f64ecd

Removed children

Add fix to remove h1 margin in PageHeader

9b39c24

CONSOLE-4492: Clean up unused CSS

9601b7e

logonoff force-pushed the pageheader branch 3 times, most recently from f88c3cb to 15036b9 Compare April 23, 2025 15:08

rhamilto reviewed Apr 23, 2025

View reviewed changes

frontend/packages/console-shared/src/components/heading/PageHeading.scss Outdated Show resolved Hide resolved

logonoff force-pushed the pageheader branch from 15036b9 to a9afa41 Compare April 23, 2025 15:17

logonoff requested a review from rhamilto April 23, 2025 15:23

rhamilto approved these changes Apr 23, 2025

View reviewed changes

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Apr 23, 2025

openshift-ci bot added the plugin-api-approved Indicates a PR with plugin API changes has been approved by an API reviewer label Apr 23, 2025

openshift-ci bot added the docs-approved Signifies that Docs has signed off on this PR label Apr 24, 2025

openshift-merge-bot bot merged commit 0ef501e into openshift:main Apr 24, 2025
8 checks passed

logonoff deleted the pageheader branch April 24, 2025 14:35

OCPBUGS-54670: Use PF component group for PageHeading #14965

OCPBUGS-54670: Use PF component group for PageHeading #14965

Uh oh!

Conversation

logonoff commented Apr 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openshift-ci-robot commented Apr 12, 2025 • edited by openshift-ci bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

logonoff commented Apr 13, 2025

Uh oh!

logonoff commented Apr 13, 2025

Uh oh!

openshift-ci-robot commented Apr 13, 2025 • edited by openshift-ci bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

yapei commented Apr 14, 2025

Uh oh!

openshift-ci-robot commented Apr 14, 2025 • edited by openshift-ci bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

logonoff commented Apr 14, 2025

Uh oh!

Uh oh!

rhamilto left a comment

Choose a reason for hiding this comment

Uh oh!

rhamilto Apr 23, 2025

Choose a reason for hiding this comment

Uh oh!

logonoff Apr 23, 2025

Choose a reason for hiding this comment

Uh oh!

rhamilto Apr 23, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

openshift-ci bot commented Apr 23, 2025

Uh oh!

logonoff commented Apr 23, 2025

Uh oh!

vojtechszocs commented Apr 23, 2025

Uh oh!

openshift-ci-robot commented Apr 23, 2025

Uh oh!

logonoff commented Apr 24, 2025

Uh oh!

openshift-ci-robot commented Apr 24, 2025

Uh oh!

rhamilto commented Apr 24, 2025

Uh oh!

jhadvig commented Apr 24, 2025

Uh oh!

openshift-ci bot commented Apr 24, 2025

Uh oh!

logonoff commented Apr 24, 2025

Uh oh!

openshift-ci bot commented Apr 24, 2025

Uh oh!

Uh oh!

openshift-ci-robot commented Apr 24, 2025

Uh oh!

openshift-merge-robot commented May 2, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

logonoff commented Apr 12, 2025 •

edited

Loading

openshift-ci-robot commented Apr 12, 2025 •

edited by openshift-ci bot

Loading

openshift-ci-robot commented Apr 13, 2025 •

edited by openshift-ci bot

Loading

openshift-ci-robot commented Apr 14, 2025 •

edited by openshift-ci bot

Loading