feat(AlertGroup): Add alert group component by jessiehuff · Pull Request #3531 · patternfly/patternfly-react · GitHub

What: Closes #3528

Additional issues: #3519 should go in first to allow for accessible toast alerts

PatternFly-React preview: https://patternfly-react-pr-3531.surge.sh

Codecov Report

❗ No coverage uploaded for pull request base (master@4dadd69). Click here to learn what that means.
The diff coverage is 88.63%.

@@            Coverage Diff            @@
##             master    #3531   +/-   ##
=========================================
  Coverage          ?   67.15%           
=========================================
  Files             ?      907           
  Lines             ?    25559           
  Branches          ?     2267           
=========================================
  Hits              ?    17165           
  Misses            ?     7353           
  Partials          ?     1041

Flag	Coverage Δ
#misc	95.45% <ø> (?)
#patternfly3	69.29% <ø> (?)
#patternfly4	64.31% <88.63%> (?)

Impacted Files	Coverage Δ
...ore/src/components/AlertGroup/AlertGroupInline.tsx	100% <100%> (ø)
...ernfly-4/react-core/src/components/Alert/Alert.tsx	84.21% <100%> (ø)
...ly-4/react-core/src/components/AlertGroup/index.ts	100% <100%> (ø)
...eact-core/src/components/AlertGroup/AlertGroup.tsx	83.33% <83.33%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4dadd69...7358826. Read the comment docs.

LGTM

I wonder if we should follow what we did in Chips/Chip Group and have Alert and Alert Groups be in the same package. @mcarrano Thoughts. I thin you had suggested the Chips be in the same package before.

Can you also had the Alert Group to the demo-app and add integration test.

@tlabaj I met with @jessiehuff and @mcoker to review this and we agreed that the following changes should be made:

Use the Inline Alert styling for examples that are other than toast alerts.
For the Additive Alert Group examples, make the alerts dismissable so you can see how alerts and both added and removed from a group.
Show the Aync Additive example with a Toast group rather than an Inline group since that's more representative of how asynchronous alerts are added to the page.

We decided to keep the packaging as is, i.e. separate Alert and Alert Group components.

@jessiehuff This is definitely looking better. Just a couple of remaining issues:

For the "Singular dynamic alert group", can we change the styling of those to inline alerts?
In the Async alert group, doesn't seem like the Stop button is working. Could not stop the alerts.

nice work @jessiehuff!! 🥇

Looks great @jessiehuff

Update look great Jessie.

I like this prop name change!

Update look great Jessie.LGTM

Your changes have been released in:

@patternfly/react-catalog-view-extension@1.2.3
@patternfly/react-core@3.134.0
@patternfly/react-docs@4.17.4
@patternfly/react-inline-edit-extension@2.15.4
demo-app-ts@3.19.0
@patternfly/react-integration@3.19.0
@patternfly/react-table@2.25.4
@patternfly/react-topology@2.12.3
@patternfly/react-virtualized-extension@1.3.91

Thanks for your contribution! 🎉

jessiehuff requested a review from seanforyou23 January 20, 2020 21:16

dlabrecq previously approved these changes Jan 21, 2020

View reviewed changes

tlabaj requested a review from mcoker January 22, 2020 15:09

tlabaj added the css review label Jan 22, 2020

redallen previously approved these changes Jan 22, 2020

View reviewed changes

jschuler suggested changes Jan 22, 2020

View reviewed changes

Comment thread packages/patternfly-4/react-core/src/components/AlertGroup/AlertGroup.tsx Outdated

tlabaj suggested changes Jan 22, 2020

View reviewed changes

Comment thread packages/patternfly-4/react-core/src/components/AlertGroup/AlertGroup.tsx Outdated

tlabaj added the ux review label Jan 22, 2020

tlabaj requested a review from mcarrano January 22, 2020 16:12

jschuler reviewed Jan 22, 2020

View reviewed changes

Comment thread packages/patternfly-4/react-core/src/components/AlertGroup/AlertGroup.tsx Outdated

jessiehuff dismissed stale reviews from redallen and dlabrecq via 0059e3f January 22, 2020 20:40

mcarrano suggested changes Jan 22, 2020

View reviewed changes

mcoker reviewed Jan 23, 2020

View reviewed changes

Comment thread packages/patternfly-4/react-core/src/components/AlertGroup/examples/alert-group.css Outdated

mcoker previously approved these changes Jan 24, 2020

View reviewed changes

jessiehuff dismissed mcoker’s stale review via 014fccd January 24, 2020 16:19

jessiehuff force-pushed the alert-group branch 2 times, most recently from e2094e5 to 9fb8c00 Compare January 24, 2020 17:50

jessiehuff added 9 commits January 24, 2020 14:23

Add alert group component

233d6fd

Fix css

8a70c9f

Update alert, edit examples, make alert group appendable

7dd6c30

Add alert group component

f055867

Fix css

4cc9ce6

Create alert group demo

0785f7d

Add integration tests

14cfaca

Fix toast example

40d566b

Make alert group accessible without alert isToast

4527ffb

jessiehuff added 4 commits January 24, 2020 14:23

Clear up difference between toast styling and a11y live regions

07ff361

Clean up tests

d345c7c

Clean up demo

2c2bbbe

Fix examples

e696b6a

jessiehuff force-pushed the alert-group branch from 5a19118 to e696b6a Compare January 24, 2020 19:24

jessiehuff requested review from mcarrano and tlabaj January 24, 2020 20:15

mcarrano previously approved these changes Jan 24, 2020

View reviewed changes

mcarrano added ux approved and removed ux review labels Jan 24, 2020

mcoker previously approved these changes Jan 24, 2020

View reviewed changes

dlabrecq previously approved these changes Jan 24, 2020

View reviewed changes

tlabaj previously approved these changes Jan 24, 2020

View reviewed changes

kmcfaul previously approved these changes Jan 24, 2020

View reviewed changes

jschuler suggested changes Jan 24, 2020

View reviewed changes

Comment thread packages/patternfly-4/react-core/src/components/Alert/Alert.tsx Outdated

Comment thread packages/patternfly-4/react-core/src/components/AlertGroup/AlertGroup.tsx

Edit append ability

c0b03f5

jessiehuff dismissed stale reviews from kmcfaul, tlabaj, dlabrecq, mcoker, and mcarrano via c0b03f5 January 27, 2020 17:30

jschuler reviewed Jan 27, 2020

View reviewed changes

Comment thread packages/patternfly-4/react-core/src/components/AlertGroup/AlertGroup.tsx Outdated

jschuler reviewed Jan 27, 2020

View reviewed changes

Comment thread packages/patternfly-4/react-core/src/components/AlertGroup/AlertGroupInline.tsx Outdated

jessiehuff added 2 commits January 27, 2020 13:18

Address feedback

8661e35

Update snapshots

7358826

jschuler approved these changes Jan 27, 2020

View reviewed changes

tlabaj approved these changes Jan 27, 2020

View reviewed changes

tlabaj merged commit 0a6806e into patternfly:master Jan 27, 2020

                     const wrapper = mount(
                       <Alert
-                        isToast={true}
+                        isLiveRegion={true}

feat(AlertGroup): Add alert group component by jessiehuff · Pull Request #3531 · patternfly/patternfly-react · GitHub

Uh oh!

feat(AlertGroup): Add alert group component - #3531

feat(AlertGroup): Add alert group component#3531tlabaj merged 16 commits intopatternfly:masterpatternfly/patternfly-react:masterfrom jessiehuff:alert-groupjessiehuff/patternfly-react:alert-groupCopy head branch name to clipboard

Conversation

jessiehuff commented Jan 20, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

patternfly-build commented Jan 20, 2020

Uh oh!

codecov-io commented Jan 20, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

dlabrecq left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

tlabaj left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

mcarrano left a comment • 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!

mcarrano commented Jan 24, 2020

Uh oh!

mcoker left a comment

Choose a reason for hiding this comment

Uh oh!

mcarrano left a comment

Choose a reason for hiding this comment

Uh oh!

tlabaj left a comment

Choose a reason for hiding this comment

Uh oh!

tlabaj Jan 24, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

tlabaj left a comment

Choose a reason for hiding this comment

Uh oh!

patternfly-build commented Jan 27, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

10 participants

feat(AlertGroup): Add alert group component#3531
tlabaj merged 16 commits into
patternfly:masterfrom
jessiehuff:alert-group

jessiehuff commented Jan 20, 2020 •
edited
Loading

codecov-io commented Jan 20, 2020 •
edited
Loading

mcarrano left a comment •
edited
Loading