Skip to content

Conversation

sg00dwin
Copy link
Member

@sg00dwin sg00dwin commented Sep 2, 2021

Removes the need for dynamic plugin css page level overrides.
Sidebar nav pf-c-page__sidebar breakpoint now uses the PF default 1200px which is tied into the default pf-c-page grid template area @media breakpoint.
A couple z-index changes so that notification drawer, fullscreen terminal, and Help menu position correctly

Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1995545 (Project drop-down hidden behind PF wizard)

Related
Dynamic Plugin demo page #9679
PF page component https://www.patternfly.org/v4/components/page
PF page with drawer demo that most closely matches our console markup. https://www.patternfly.org/v4/components/drawer/html-demos#expanded because of how we pull in the ConnectedNotificationDrawer

@spadgett
Copy link
Member

spadgett commented Sep 2, 2021

@jamestalton FYI

Copy link
Member

@rhamilto rhamilto left a comment

Choose a reason for hiding this comment

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

One nit.

What's your confidence on this, @sg00dwin?

@sg00dwin sg00dwin changed the title Console 2926: Update console page markup structure to closer align with Patternfly page component structure. [WIP] Console 2926: Update console page markup structure to closer align with Patternfly page component structure. Sep 2, 2021
@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 2, 2021
@sg00dwin sg00dwin force-pushed the app-page-restructure-for-better-alignment-with-PF branch from 324c20c to 39419bf Compare September 2, 2021 21:56
@sg00dwin
Copy link
Member Author

sg00dwin commented Sep 2, 2021

One nit.

What's your confidence on this, @sg00dwin?

Removed after our conversation

@rexagod
Copy link
Member

rexagod commented Sep 6, 2021

/bugzilla refresh

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Sep 6, 2021

@rexagod: No Bugzilla bug is referenced in the title of this pull request.
To reference a bug, add 'Bug XXX:' to the title of this pull request and request another bug refresh with /bugzilla refresh.

In response to this:

/bugzilla refresh

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/test-infra repository.

@sg00dwin sg00dwin force-pushed the app-page-restructure-for-better-alignment-with-PF branch from 39419bf to ab620da Compare September 7, 2021 13:40
@sg00dwin sg00dwin changed the title [WIP] Console 2926: Update console page markup structure to closer align with Patternfly page component structure. Console 2926: Bug 1995545: Update console page markup structure to closer align with Patternfly page component structure. Sep 7, 2021
@openshift-ci openshift-ci bot added bugzilla/severity-low Referenced Bugzilla bug's severity is low for the branch this PR is targeting. bugzilla/invalid-bug Indicates that a referenced Bugzilla bug is invalid for the branch this PR is targeting. and removed do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. labels Sep 7, 2021
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Sep 7, 2021

@sg00dwin: This pull request references Bugzilla bug 1995545, which is invalid:

  • expected the bug to target the "4.10.0" release, but it targets "4.9.0" instead

Comment /bugzilla refresh to re-evaluate validity if changes to the Bugzilla bug are made, or edit the title of this pull request to link to a different bug.

In response to this:

Console 2926: Bug 1995545: Update console page markup structure to closer align with Patternfly page component structure.

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/test-infra repository.

@sg00dwin
Copy link
Member Author

sg00dwin commented Sep 7, 2021

/bugzilla refresh

@openshift-ci openshift-ci bot added bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. and removed bugzilla/invalid-bug Indicates that a referenced Bugzilla bug is invalid for the branch this PR is targeting. labels Sep 7, 2021
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Sep 7, 2021

@sg00dwin: This pull request references Bugzilla bug 1995545, which is valid. The bug has been moved to the POST state.

3 validation(s) were run on this bug
  • bug is open, matching expected state (open)
  • bug target release (4.10.0) matches configured target release for branch (4.10.0)
  • bug is in the state NEW, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

No GitHub users were found matching the public email listed for the QA contact in Bugzilla ([email protected]), skipping review request.

In response to this:

/bugzilla refresh

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/test-infra repository.

@sg00dwin sg00dwin force-pushed the app-page-restructure-for-better-alignment-with-PF branch 3 times, most recently from aa4bcc4 to b6f5fb8 Compare September 14, 2021 20:54
@openshift-ci openshift-ci bot added the kind/cypress Related to Cypress e2e integration testing label Sep 14, 2021
@sg00dwin
Copy link
Member Author

/retest

@sg00dwin sg00dwin force-pushed the app-page-restructure-for-better-alignment-with-PF branch 2 times, most recently from 480bfbf to 79a3a68 Compare September 16, 2021 19:56
@openshift-ci openshift-ci bot added the component/ceph Related to ceph-storage-plugin label Sep 16, 2021
@openshift-ci openshift-ci bot added the component/olm Related to OLM label Sep 16, 2021
@dtaylor113
Copy link
Contributor

/retest

@sg00dwin
Copy link
Member Author

/test ceph-storage-plugin

Copy link
Member

@rhamilto rhamilto left a comment

Choose a reason for hiding this comment

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

The scrollbar now overlaps with areas that are not scrollable. I assume this is expected given the sticky positioning of the global notifications and project picker, but wanted to point it out in the event it is unexpected.
Screen Shot 2021-09-17 at 2 05 44 PM

…verrides are needed and

do not interfere with default PF component usage.
Relates to dynamic plugin override issues
@sg00dwin sg00dwin force-pushed the app-page-restructure-for-better-alignment-with-PF branch from 79a3a68 to b276e3d Compare September 22, 2021 14:46
Copy link
Member

@rhamilto rhamilto 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 22, 2021
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Sep 22, 2021

[APPROVALNOTIFIER] This PR is APPROVED

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

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

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 22, 2021
@openshift-bot
Copy link
Contributor

/retest-required

Please review the full test history for this PR and help us cut down flakes.

1 similar comment
@openshift-bot
Copy link
Contributor

/retest-required

Please review the full test history for this PR and help us cut down flakes.

@openshift-merge-robot openshift-merge-robot merged commit c6923b9 into openshift:master Sep 23, 2021
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Sep 23, 2021

@sg00dwin: All pull requests linked via external trackers have merged:

Bugzilla bug 1995545 has been moved to the MODIFIED state.

In response to this:

Console 2926: Bug 1995545: Update console page markup structure to closer align with Patternfly page component structure.

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/test-infra repository.

@sg00dwin sg00dwin deleted the app-page-restructure-for-better-alignment-with-PF branch September 23, 2021 14:43
@spadgett spadgett added this to the v4.10 milestone Sep 23, 2021
gouyang pushed a commit to gouyang/console that referenced this pull request Sep 25, 2021
…for-better-alignment-with-PF

Console 2926: Bug 1995545: Update console page markup structure to closer align with Patternfly page component structure.
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. bugzilla/severity-low Referenced Bugzilla bug's severity is low for the branch this PR is targeting. bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. component/ceph Related to ceph-storage-plugin component/core Related to console core functionality component/olm Related to OLM kind/cypress Related to Cypress e2e integration testing lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants