Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(DataToolbar): move and wrap chips in expandable content #3319

Merged
merged 6 commits into from Nov 21, 2019

Conversation

@nicolethoen
Copy link
Contributor

nicolethoen commented Nov 19, 2019

Fixes #3198

follow up to Core issue #2397

@patternfly-build

This comment has been minimized.

Copy link
Contributor

patternfly-build commented Nov 19, 2019

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

@codecov-io

This comment has been minimized.

Copy link

codecov-io commented Nov 19, 2019

Codecov Report

Merging #3319 into master will decrease coverage by 2.35%.
The diff coverage is 85.18%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3319      +/-   ##
==========================================
- Coverage   67.44%   65.09%   -2.36%     
==========================================
  Files         892      485     -407     
  Lines       24874    11828   -13046     
  Branches     2141     2143       +2     
==========================================
- Hits        16776     7699    -9077     
+ Misses       7093     3123    -3970     
- Partials     1005     1006       +1
Flag Coverage Δ
#misc 95.45% <ø> (ø) ⬆️
#patternfly3 ?
#patternfly4 64.8% <85.18%> (+0.02%) ⬆️
Impacted Files Coverage Δ
...mental/components/DataToolbar/DataToolbarUtils.tsx 100% <ø> (ø) ⬆️
...onents/DataToolbar/DataToolbarChipGroupContent.tsx 81.81% <ø> (ø) ⬆️
...nents/DataToolbar/DataToolbarExpandableContent.tsx 96.29% <100%> (+1.05%) ⬆️
...mental/components/DataToolbar/DataToolbarGroup.tsx 100% <100%> (+6.25%) ⬆️
...ntal/components/DataToolbar/DataToolbarContent.tsx 100% <100%> (ø) ⬆️
...xperimental/components/DataToolbar/DataToolbar.tsx 79.24% <40%> (-2.76%) ⬇️
...ental/components/DataToolbar/DataToolbarFilter.tsx 87.5% <80%> (-2.16%) ⬇️
...ernfly-react/src/components/LoginPage/LoginPage.js 37.73% <0%> (-51.16%) ⬇️
...nfly-3/patternfly-react/src/components/Tabs/Tab.js 57.14% <0%> (-31.75%) ⬇️
...fly-3/patternfly-react/src/components/Form/Form.js 57.69% <0%> (-29.27%) ⬇️
... and 458 more

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 e5df009...7296d44. Read the comment docs.

Copy link
Contributor

mattnolting left a comment

Screen Shot 2019-11-19 at 12 06 23 PM

There's an extra __group in __expandable-content

Screen Shot 2019-11-19 at 12 16 33 PM

,pf-c-data-toolbar__group.pf-m-chip-container needs to be hidden when nothing is selected

@nicolethoen

This comment has been minimized.

Copy link
Contributor Author

nicolethoen commented Nov 19, 2019

There's an extra __group in __expandable-content

@mattnolting yes, that is a container that lets me cheat react and 'prepend' the components to display in the expandable content BEFORE the chip-container...

@tlabaj tlabaj added the css review label Nov 19, 2019
Copy link
Contributor

tlabaj left a comment

You probably want to update the demo app to use the new prop.

Copy link
Contributor

tlabaj left a comment

Hi Nicole. I was talking about updating the integration demo-app and updating the cypress test if applicable.

@nicolethoen

This comment has been minimized.

Copy link
Contributor Author

nicolethoen commented Nov 20, 2019

@tlabaj ah sure. I'll update that

@mattnolting

This comment has been minimized.

Copy link
Contributor

mattnolting commented Nov 20, 2019

@mattnolting yes, that is a container that lets me cheat react and 'prepend' the components to display in the expandable content BEFORE the chip-container...

Ok, I'll need to make one small CSS update to remove margin values. Not a blocker.

nicolethoen added 2 commits Nov 20, 2019
Copy link
Member

mcarrano left a comment

Looks good @nicolethoen

@tlabaj
tlabaj approved these changes Nov 20, 2019
Copy link
Contributor

tlabaj left a comment

LGTM

@dlabaj
dlabaj approved these changes Nov 21, 2019
@mattnolting

This comment has been minimized.

Copy link
Contributor

mattnolting commented Nov 21, 2019

PERFECT🥇

@tlabaj tlabaj merged commit 99f1efe into patternfly:master Nov 21, 2019
8 checks passed
8 checks passed
ci/circleci: build Your tests passed on CircleCI!
Details
ci/circleci: build_integration Your tests passed on CircleCI!
Details
ci/circleci: build_pf3_docs Your tests passed on CircleCI!
Details
ci/circleci: build_pf4_docs Your tests passed on CircleCI!
Details
ci/circleci: lint Your tests passed on CircleCI!
Details
ci/circleci: test_jest_other Your tests passed on CircleCI!
Details
ci/circleci: test_jest_pf4 Your tests passed on CircleCI!
Details
ci/circleci: upload_docs Your tests passed on CircleCI!
Details
@patternfly-build

This comment has been minimized.

Copy link
Contributor

patternfly-build commented Nov 21, 2019

Your changes have been released in:

  • @patternfly/react-catalog-view-extension@1.1.36
  • @patternfly/react-core@3.123.1
  • @patternfly/react-docs@4.16.42
  • @patternfly/react-inline-edit-extension@2.13.7
  • demo-app-ts@3.12.1
  • @patternfly/react-integration@3.12.1
  • @patternfly/react-table@2.24.39
  • @patternfly/react-topology@2.11.25
  • @patternfly/react-virtualized-extension@1.3.38

Thanks for your contribution! 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
8 participants
You can’t perform that action at this time.