Skip to content

Conversation

@dlabaj
Copy link
Contributor

@dlabaj dlabaj commented Apr 26, 2019

Fixes the behavior of the accordion for #1387 and makes accordion available to consumers.

Additional issues:
Please merge this PR first #1852 (review) I'll rebase after to get these in.

@patternfly-build
Copy link
Collaborator

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

@codecov-io
Copy link

codecov-io commented Apr 26, 2019

Codecov Report

Merging #1860 into master will decrease coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1860      +/-   ##
==========================================
- Coverage   82.59%   82.59%   -0.01%     
==========================================
  Files         621      622       +1     
  Lines        6844     6849       +5     
  Branches       93       93              
==========================================
+ Hits         5653     5657       +4     
- Misses       1151     1152       +1     
  Partials       40       40
Flag Coverage Δ
#patternfly3 84.87% <ø> (ø) ⬆️
#patternfly4 79.29% <100%> (ø) ⬆️
#patternflymisc 95.68% <ø> (ø) ⬆️
Impacted Files Coverage Δ
...t-core/src/components/Accordion/AccordionToggle.js 100% <100%> (ø) ⬆️
...ct-table/src/components/Table/utils/headerUtils.js 100% <0%> (ø) ⬆️
...ernfly-4/react-table/src/components/Table/Table.js 88.46% <0%> (ø) ⬆️
...t-table/src/components/Table/utils/transformers.js 100% <0%> (ø) ⬆️
...rc/components/Table/utils/decorators/classNames.js 75% <0%> (ø)

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 4ae95e9...2ff69e7. Read the comment docs.

mcoker
mcoker previously approved these changes Apr 26, 2019
mcarrano
mcarrano previously approved these changes Apr 26, 2019
Copy link
Member

@mcarrano mcarrano left a comment

Choose a reason for hiding this comment

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

This now behaves as I expect. Thanks @dlabaj !

tlabaj
tlabaj previously approved these changes Apr 26, 2019
Copy link
Contributor

@tlabaj tlabaj left a comment

Choose a reason for hiding this comment

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

LGTM

kmcfaul
kmcfaul previously approved these changes Apr 26, 2019
@dlabaj dlabaj dismissed stale reviews from kmcfaul, tlabaj, mcarrano, and mcoker via 0127879 April 26, 2019 20:13
jessiehuff
jessiehuff previously approved these changes Apr 26, 2019
Copy link
Contributor

@jessiehuff jessiehuff left a comment

Choose a reason for hiding this comment

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

LGTM!

dgutride
dgutride previously approved these changes Apr 26, 2019
tlabaj
tlabaj previously approved these changes Apr 26, 2019
Copy link
Contributor

@tlabaj tlabaj left a comment

Choose a reason for hiding this comment

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

LGTM

@mcoker
Copy link
Contributor

mcoker commented Apr 26, 2019

@dlabaj can you get rid of the span that wraps the svg? It's not necessary.

Screen Shot 2019-04-26 at 3 36 46 PM

@dlabaj
Copy link
Contributor Author

dlabaj commented Apr 26, 2019

@mcoker sure thing. Doing it now.

@dlabaj dlabaj dismissed stale reviews from tlabaj, dgutride, and jessiehuff via 2ff69e7 April 26, 2019 20:43
Copy link
Contributor

@tlabaj tlabaj left a comment

Choose a reason for hiding this comment

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

LGTM

@tlabaj tlabaj merged commit 6db9335 into patternfly:master Apr 26, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants