Skip to content

Conversation

@rhamilto
Copy link
Member

@rhamilto rhamilto commented Feb 27, 2025

  • Replace co-m-page__body with PatternFly's Flex component
  • Use PatternFly's PageBreadcrumb to position breadcrumbs
  • Replace div#app-content with PatternFly's Flex component
  • Replace co-m-pane__body and co-m-nav-title with PatternFly's PageSection component

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

openshift-ci-robot commented Feb 27, 2025

@rhamilto: 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:

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 the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Feb 27, 2025
@openshift-ci openshift-ci bot added component/core Related to console core functionality component/dashboard Related to dashboard approved Indicates a PR has been approved by an approver from all required OWNERS files. component/dev-console Related to dev-console component/gitops Related to gitops-plugin component/helm Related to helm-plugin component/knative Related to knative-plugin labels Feb 27, 2025
@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented Feb 27, 2025

@rhamilto: 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:

  • Replace co-m-page__body with PatternFly's Flex component
  • Use PatternFly's PageBreadcrumb to position breadcrumbs
  • Replace div#app-content with PatternFly's Flex component
  • Replace co-m-pane__body and co-m-nav-title with PatternFly's PageSection component

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/lso Related to local-storage-operator-plugin component/metal3 Related to metal3-plugin component/monitoring Related to monitoring component/olm Related to OLM component/pipelines Related to pipelines-plugin component/sdk Related to console-plugin-sdk component/shared Related to console-shared component/topology Related to topology labels Feb 27, 2025
Copy link
Member Author

@rhamilto rhamilto Feb 27, 2025

Choose a reason for hiding this comment

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

Remove divider so the page layout is consistent with other pages in Home.

Copy link
Member Author

Choose a reason for hiding this comment

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

Removing detail={true} removes the divider between the header and the page contents so the layout is consistent with other pages in Home.

Copy link
Member Author

Choose a reason for hiding this comment

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

Not needed.

Copy link
Member Author

Choose a reason for hiding this comment

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

Align values with PatternFly 6.

Copy link
Member Author

Choose a reason for hiding this comment

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

PatternFly's <Tabs> does not have an inset value that perfectly aligns the inset with the padding of <PageSection>. This is a bug? @nicolethoen, @kmcfaul

Copy link
Member

Choose a reason for hiding this comment

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

I'm asking about that. will report back

Copy link
Member

Choose a reason for hiding this comment

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

I'm being told it is a bug 👍🏻
Would you mind opening a bug in https://github.com/patternfly/patternfly/issues?
You can link to it in this override.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done. Will also add a link to the bug here.

Copy link
Member Author

Choose a reason for hiding this comment

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

Relocating to the correct partial.

@rhamilto
Copy link
Member Author

rhamilto commented Feb 27, 2025

@logonoff, I've got an additional commit to update docs, but don't want to interrupt the current CI run by pushing it. Posting the diff here for your viewing pleasure. ;-)

doc.patch

@rhamilto
Copy link
Member Author

/assign @logonoff
/assign @XiyunZhao

Follow-on to previously approved story, so assigning approvals:
/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 Feb 27, 2025
Copy link
Member Author

@rhamilto rhamilto Feb 27, 2025

Choose a reason for hiding this comment

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

Add the border above the tabs since it lacks them and the standard is for pages with tabs to have the border.

Screenshot 2025-02-27 at 2 39 32 PM

Copy link
Member

@logonoff logonoff Mar 3, 2025

Choose a reason for hiding this comment

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

To better match with PF I think that the PageHeading border should be removed here as well. This would better match with the PF mockup that was made for OCP as well:

image

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh. I missed this one. I think I agree. Gonna discuss this one with the broader community.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thinking we should tackle this as a follow on so as not to conflate the issues.

@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Mar 24, 2025
@logonoff
Copy link
Member

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Mar 24, 2025
@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Mar 24, 2025
@logonoff
Copy link
Member

/lgtm

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

openshift-ci bot commented Mar 24, 2025

[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:

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

@logonoff
Copy link
Member

/label acknowledge-critical-fixes-only

qe approval:

/assign @yapei

@openshift-ci openshift-ci bot added the acknowledge-critical-fixes-only Indicates if the issuer of the label is OK with the policy. label Mar 24, 2025
@rhamilto
Copy link
Member Author

/retest

1 similar comment
@rhamilto
Copy link
Member Author

/retest

@logonoff
Copy link
Member

/close

superseded by #14875

@openshift-ci openshift-ci bot closed this Mar 25, 2025
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Mar 25, 2025

@logonoff: Closed this PR.

Details

In response to this:

/close

superseded by #14875

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

acknowledge-critical-fixes-only Indicates if the issuer of the label is OK with the policy. approved Indicates a PR has been approved by an approver from all required OWNERS files. component/core Related to console core functionality component/dashboard Related to dashboard component/dev-console Related to dev-console component/gitops Related to gitops-plugin component/helm Related to helm-plugin component/knative Related to knative-plugin component/lso Related to local-storage-operator-plugin component/metal3 Related to metal3-plugin component/monitoring Related to monitoring component/olm Related to OLM component/pipelines Related to pipelines-plugin component/sdk Related to console-plugin-sdk component/shared Related to console-shared component/topology Related to topology 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 tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants