Card Demos: Aggregate Status, Status Tabbed, Status by CooperRedhat · Pull Request #6206 · patternfly/patternfly-react

CooperRedhat · 2021-08-20T16:33:03Z

What: Closes #6189
Closes #6207
Closes #6217
Closes #6219
Closes #6220

Not directly related to this issue, but NotificationDrawerGroup was updated to handle more than a string title in order to follow examples in core

patternfly-build · 2021-08-20T16:39:35Z

PF4 preview: https://patternfly-react-pr-6206.surge.sh
A11y report: https://patternfly-react-pr-6206-a11y.surge.sh

packages/react-core/src/demos/examples/CardDemos/Aggregated.tsx

packages/react-core/src/demos/CardDemos.md

mcarrano

This looks good @CooperRedhat but I just had some questions about the tabbed status card demo. I'm see some alignment that does not feel quite right when I compare with core version.

Note that the text "Ready" seems misaligned.

I also wonder about this example, @mattnolting do you remember where it came from? I know that this is only a demo, but using the (!) icon which a label that reads Ready seems contradictory. Maybe we should be using the green check (success) icon here?

CooperRedhat · 2021-08-30T19:50:40Z

This looks good @CooperRedhat but I just had some questions about the tabbed status card demo. I'm see some alignment that does not feel quite right when I compare with core version.

Note that the text "Ready" seems misaligned.

I also wonder about this example, @mattnolting do you remember where it came from? I know that this is only a demo, but using the (!) icon which a label that reads Ready seems contradictory. Maybe we should be using the green check (success) icon here?

@mcarrano: @nicolethoen noticed the same issue and we're addressing it, thanks!

mcoker · 2021-09-01T19:24:58Z

@mcarrano: @nicolethoen noticed the same issue and we're addressing it, thanks!

FWIW, it looks like vertical-align: -0.125em; fixes it. I see that applied to a lot of icons we use (for example in the alert component).

CooperRedhat · 2021-09-01T19:31:29Z

@mcarrano: @nicolethoen noticed the same issue and we're addressing it, thanks!

FWIW, it looks like vertical-align: -0.125em; fixes it. I see that applied to a lot of icons we use (for example in the alert component).

FYI (and my own clarity) I think there are two issues here, @mcarrano was talking about Ready and Running being misaligned, which I pushed a fix for.

@mcoker You are referring to something else, which is text/icon alignment, correct?

mcoker · 2021-09-01T19:32:58Z

@mcoker You are referring to something else, which is text/icon alignment, correct?

@CooperRedhat oh yep, sorry. This for example - I applied vertical-align to the icon on the left but not on the right

mattnolting

Looks great! Just a few updates.

mattnolting · 2021-09-09T17:43:49Z

packages/react-core/src/demos/CardDemos.md

+        status: 'Running',
+        resourceName: 'Resource name that is long and can wrap',
+        detail: '121 Systems',
+        icon: <Spinner size='md' aria-valuetext='Loading...'></Spinner>


Let's replace this with a check icon

They core demo has spinners. Will that be updated as well?

Changed the spinners to checks, leaving this conversation open for @tlabaj's comment.

mattnolting · 2021-09-09T17:44:02Z

packages/react-core/src/demos/CardDemos.md

+        status: 'Running',
+        resourceName: 'Resource name that is long and can wrap',
+        detail: '122 Systems',
+        icon: <Spinner size='md' aria-valuetext='Loading...'></Spinner>


Replace this with a check icon

tlabaj

Looks great!

mattnolting

LPTM 👍

tlabaj · 2021-09-10T20:48:00Z

packages/react-core/src/demos/CardDemos.md

+                <a href="#">Image Vulnerabilities</a>
+              </FlexItem>
+              <FlexItem>
+                <span style={{ color: '#8a8d90' }}>0 vulnerabilities</span>


Does this one also need to use a var?

mcarrano

Looks much better. Thanks @CooperRedhat !

patternfly-build · 2021-09-13T14:15:00Z

Your changes have been released in:

@patternfly/react-catalog-view-extension@4.12.65
@patternfly/react-code-editor@4.3.52
@patternfly/react-console@4.11.33
@patternfly/react-core@4.155.2
@patternfly/react-docs@5.21.31
@patternfly/react-inline-edit-extension@4.7.75
demo-app-ts@4.120.2
@patternfly/react-log-viewer@4.6.5
@patternfly/react-table@4.29.68
@patternfly/react-topology@4.9.71
@patternfly/react-virtualized-extension@4.9.40

Thanks for your contribution! 🎉

demos(Card): add aggregate status card

d7f507f

demos(Card): add status tabbed card

cc679c9

CooperRedhat changed the title ~~demos(Card): add aggregate status card~~ Card Demos: Aggregate Status, Status Tabbed Aug 20, 2021

tlabaj added ux review labels Aug 23, 2021

tlabaj linked an issue Aug 25, 2021 that may be closed by this pull request

Card: Status, tabbed card demo #6207

Closed

chore(Card): add status, status tabbed demos

ac76157

CooperRedhat changed the title ~~Card Demos: Aggregate Status, Status Tabbed~~ Card Demos: Aggregate Status, Status Tabbed, Status Aug 25, 2021

This was linked to issues Aug 25, 2021

Card: Status demo #6217

Closed

Card: Status card expanded with popover demo #6220

Closed

Merge branch 'main' into cooper-card-demo

0de207e

redallen reviewed Aug 26, 2021

View reviewed changes

packages/react-core/src/demos/examples/CardDemos/Aggregated.tsx Outdated Show resolved Hide resolved

CooperRedhat added 2 commits August 26, 2021 14:17

fix(Card): fix doc build errors

8c117da

fix(Card): move demos to parent MD file

7873bfe

nicolethoen requested review from kmcfaul, mattnolting, mcarrano and nicolethoen August 30, 2021 15:34

nicolethoen reviewed Aug 30, 2021

View reviewed changes

mcarrano reviewed Aug 30, 2021

View reviewed changes

fix(Card): use css vars, alignment issues

1136cc6

CooperRedhat requested review from mcarrano and nicolethoen August 31, 2021 18:44

fix(Card): icon alignment

14d7426

mattnolting reviewed Sep 9, 2021

View reviewed changes

tlabaj previously approved these changes Sep 9, 2021

View reviewed changes

fix(Card): change spinner to check

9f1ca88

CooperRedhat dismissed tlabaj’s stale review via 9f1ca88 September 10, 2021 13:07

fix(Card): dup keys and a11y

2bef36d

mattnolting previously approved these changes Sep 10, 2021

View reviewed changes

fix(Card): a11y

ff4b2f8

CooperRedhat dismissed mattnolting’s stale review via ff4b2f8 September 10, 2021 19:41

tlabaj reviewed Sep 10, 2021

View reviewed changes

mcarrano approved these changes Sep 10, 2021

View reviewed changes

tlabaj approved these changes Sep 13, 2021

View reviewed changes

nicolethoen approved these changes Sep 13, 2021

View reviewed changes

nicolethoen merged commit 4f39159 into patternfly:main Sep 13, 2021

Conversation

CooperRedhat commented Aug 20, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

patternfly-build commented Aug 20, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mcarrano left a comment

Choose a reason for hiding this comment

Uh oh!

CooperRedhat commented Aug 30, 2021

Uh oh!

mcoker commented Sep 1, 2021

Uh oh!

CooperRedhat commented Sep 1, 2021

Uh oh!

mcoker commented Sep 1, 2021

Uh oh!

mattnolting left a comment

Choose a reason for hiding this comment

Uh oh!

mattnolting Sep 9, 2021

Choose a reason for hiding this comment

Uh oh!

tlabaj Sep 9, 2021

Choose a reason for hiding this comment

Uh oh!

CooperRedhat Sep 10, 2021

Choose a reason for hiding this comment

Uh oh!

mattnolting Sep 9, 2021

Choose a reason for hiding this comment

Uh oh!

tlabaj left a comment

Choose a reason for hiding this comment

Uh oh!

mattnolting left a comment

Choose a reason for hiding this comment

Uh oh!

tlabaj Sep 10, 2021

Choose a reason for hiding this comment

Uh oh!

mcarrano left a comment

Choose a reason for hiding this comment

Uh oh!

patternfly-build commented Sep 13, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

CooperRedhat commented Aug 20, 2021 •

edited

Loading

patternfly-build commented Aug 20, 2021 •

edited

Loading