Convert dropdowns and related inputs to pf4 #1751

sg00dwin · 2019-06-18T20:31:23Z

No description provided.

spadgett · 2019-06-18T20:39:41Z

cc @christianvogt

spadgett · 2019-06-18T20:42:02Z

cc @priley86

rhamilto

Nice job, @sg00dwin. A few nits, suggestions.

frontend/public/components/_cluster-service-instance.scss

frontend/public/components/_deploy-image.scss

frontend/public/components/_dropdown.scss

frontend/public/components/_source-to-image.scss

frontend/public/components/cluster-settings/_cluster-settings.scss

rhamilto · 2019-06-19T12:54:00Z

frontend/public/style/_overrides.scss

Moving these lines out of this file breaks our convention of putting overrides of third-party css in a single file and results in us having overrides throughout. I'm not opposed to that, but do favor consistency.

Turns out we are not overriding existing react-tagsinput

frontend/public/vendor.scss

rhamilto · 2019-06-19T12:55:31Z

frontend/public/style/_overrides.scss

priley86 · 2019-06-19T14:12:17Z

these dropdowns look a lot more consistent to me now, nice work @sg00dwin!

Just noting a few things after glancing at this and starting #1629 previously (not sure if they were already noted):

Events and Search dropdown items could probably use some design adjustment. cc: @beanh66

The CSS override block here can ideally be removed completely after this PR. I added this temporarily while retroverting the PF4 Kebab in patternfly react-tables #1465
https://github.com/openshift/console/blob/master/frontend/public/style/_overrides.scss#L603-L634

sg00dwin · 2019-06-19T15:59:07Z

these dropdowns look a lot more consistent to me now, nice work @sg00dwin!

Just noting a few things after glancing at this and starting #1629 previously (not sure if they were already noted):
* Events and Search dropdown items could probably use some design adjustment. cc: @beanh66
* The CSS override block here can ideally be removed completely after this PR. I added this temporarily while retroverting the PF4 Kebab in #1465
  https://github.com/openshift/console/blob/master/frontend/public/style/_overrides.scss#L603-L634

thanks @priley86 those dropdowns where missing a rule to control min-width. And I'll address those kebab overrides.

sg00dwin · 2019-06-19T20:51:34Z

@rhamilto Addressed comments ptal

Corrected Events and Search dropdown-menu widths
Right alignment of kebab dropdown-menu with list table
Alphabetize css rules in _overrides.scss

frontend/public/vendor.scss

rhamilto · 2019-06-20T12:40:14Z

/retest

sg00dwin · 2019-06-20T15:25:25Z

/retest

rhamilto

/lgtm

openshift-bot · 2019-06-20T16:33:38Z

/retest

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

spadgett · 2019-06-24T15:44:21Z

frontend/public/style/_overrides.scss

What impact does this have if we change this globally to $font-size-base as we transition to PF4? Does it break the nav/masthead? The dev console team is looking at bringing in some other PF4 components and will have similar font-size issues.

cc @rhamilto @christianvogt

It would be nice if we can override --pf-global--FontSize--md and set it to 14px so that every new PF4 component we utilize doesn't need a separate font tweak.

@spadgett Setting --pf-global--FontSize--md to $font-size-base: 14px globally still requires a few targeted overrides. Can we make those global font-size changes as a follow-on and go forward with merging this pf, so the font related changes can be easier tracked?

@sg00dwin I was going to suggest a separate PR, yeah. Then we can get that merged separately to unblock the dev console team. It looks like you have some test failures to work through still in this one.

beanh66 · 2019-06-25T14:10:55Z

@sg00dwin LGTM!

@lizsurette @serenamarie125 FYI

lizsurette · 2019-06-25T15:19:44Z

Very nice!

FYI @matthewcarleton

sg00dwin · 2019-06-26T14:41:16Z

/retest

matthewcarleton · 2019-06-26T15:55:34Z

looks great!

rhamilto · 2019-06-27T14:14:28Z

/retest

rhamilto · 2019-06-27T16:54:45Z

/lgtm

spadgett · 2019-06-27T17:02:03Z

frontend/integration-tests/views/namespace.view.ts

FYI, This will conflict with #1542

frontend/integration-tests/tests/crud.scenario.ts

openshift-bot · 2019-06-27T17:58:08Z

/retest

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

rhamilto · 2019-06-27T18:38:41Z

/lgtm

openshift-ci-robot · 2019-06-27T18:38:51Z

[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

Details

Needs approval from an approver in each of these files:

~~frontend/OWNERS~~ [rhamilto]

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

openshift-bot · 2019-06-27T19:42:11Z

/retest

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

rhamilto · 2019-06-27T20:46:58Z

/retest

openshift-bot · 2019-06-27T20:47:05Z

/retest

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

sg00dwin requested review from rhamilto and spadgett June 18, 2019 20:31

openshift-ci-robot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Jun 18, 2019

openshift-ci-robot requested a review from dtaylor113 June 18, 2019 20:31

rhamilto reviewed Jun 19, 2019

View reviewed changes

priley86 mentioned this pull request Jun 19, 2019

convert dropdown markup to pf4 #1629

Closed

spadgett added this to the v4.2 milestone Jun 19, 2019

sg00dwin force-pushed the pf4-dropdown-merge branch from 35bab63 to 6f499c5 Compare June 19, 2019 19:59

openshift-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Jun 19, 2019

sg00dwin force-pushed the pf4-dropdown-merge branch 3 times, most recently from d2a0691 to 8910134 Compare June 19, 2019 20:47

openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 19, 2019

sg00dwin force-pushed the pf4-dropdown-merge branch from 8910134 to 47367da Compare June 19, 2019 21:27

openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 19, 2019

rhamilto reviewed Jun 20, 2019

View reviewed changes

frontend/public/vendor.scss Outdated Show resolved Hide resolved

sg00dwin force-pushed the pf4-dropdown-merge branch from 47367da to 222fca9 Compare June 20, 2019 14:13

rhamilto reviewed Jun 20, 2019

View reviewed changes

openshift-ci-robot assigned rhamilto Jun 20, 2019

openshift-ci-robot added lgtm Indicates that a PR is ready to be merged. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Jun 20, 2019

openshift-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jun 24, 2019

spadgett reviewed Jun 24, 2019

View reviewed changes

sg00dwin force-pushed the pf4-dropdown-merge branch 2 times, most recently from 664ee32 to 4254de9 Compare June 25, 2019 13:51

sg00dwin force-pushed the pf4-dropdown-merge branch 2 times, most recently from 27d9f01 to 6d5b4ff Compare June 26, 2019 14:34

sg00dwin force-pushed the pf4-dropdown-merge branch 2 times, most recently from d9a59f6 to e0382c9 Compare June 26, 2019 17:54

sg00dwin force-pushed the pf4-dropdown-merge branch 2 times, most recently from a557961 to c1b69e0 Compare June 27, 2019 16:33

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Jun 27, 2019

spadgett reviewed Jun 27, 2019

View reviewed changes

Convert dropdowns and related inputs to pf4

ca935b2

sg00dwin force-pushed the pf4-dropdown-merge branch from c1b69e0 to ca935b2 Compare June 27, 2019 18:36

openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Jun 27, 2019

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Jun 27, 2019

openshift-merge-robot merged commit 2fc10f8 into openshift:master Jun 27, 2019

sg00dwin deleted the pf4-dropdown-merge branch December 9, 2019 21:07

Convert dropdowns and related inputs to pf4 #1751

Convert dropdowns and related inputs to pf4 #1751

Uh oh!

Conversation

sg00dwin commented Jun 18, 2019

Uh oh!

spadgett commented Jun 18, 2019

Uh oh!

spadgett commented Jun 18, 2019

Uh oh!

rhamilto left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

rhamilto Jun 19, 2019

Choose a reason for hiding this comment

Uh oh!

sg00dwin Jun 19, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

rhamilto Jun 19, 2019

Choose a reason for hiding this comment

Uh oh!

priley86 commented Jun 19, 2019

Uh oh!

sg00dwin commented Jun 19, 2019

Uh oh!

sg00dwin commented Jun 19, 2019

Uh oh!

Uh oh!

rhamilto commented Jun 20, 2019

Uh oh!

sg00dwin commented Jun 20, 2019

Uh oh!

rhamilto left a comment

Choose a reason for hiding this comment

Uh oh!

openshift-bot commented Jun 20, 2019

Uh oh!

spadgett Jun 24, 2019

Choose a reason for hiding this comment

Uh oh!

christianvogt Jun 24, 2019

Choose a reason for hiding this comment

Uh oh!

sg00dwin Jun 24, 2019

Choose a reason for hiding this comment

Uh oh!

spadgett Jun 24, 2019

Choose a reason for hiding this comment

Uh oh!

beanh66 commented Jun 25, 2019

Uh oh!

lizsurette commented Jun 25, 2019

Uh oh!

sg00dwin commented Jun 26, 2019

Uh oh!

matthewcarleton commented Jun 26, 2019

Uh oh!

rhamilto commented Jun 27, 2019

Uh oh!

rhamilto commented Jun 27, 2019

Uh oh!

spadgett Jun 27, 2019

Choose a reason for hiding this comment

Uh oh!

Uh oh!

openshift-bot commented Jun 27, 2019

Uh oh!

rhamilto commented Jun 27, 2019

Uh oh!

openshift-ci-robot commented Jun 27, 2019

sg00dwin Jun 19, 2019 •

edited

Loading