FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

chore(charts): adds charts to react-integration tests by jenny-s51 · Pull Request #2354 · patternfly/patternfly-react · GitHub

/ patternfly-react Public

chore(charts): adds charts to react-integration tests - #2354

Merged
dlabaj merged 14 commits into
patternfly:masterfrom
jenny-s51:iss2231
Jul 3, 2019
ConversationCommits (14)ChecksFiles changed
Merged

chore(charts): adds charts to react-integration tests#2354
dlabaj merged 14 commits into
patternfly:masterfrom
jenny-s51:iss2231

Conversation

jenny-s51 commented Jun 25, 2019

Copy link
Copy Markdown
Contributor

What: closes #2231 And adds charts to react-integration. This PR includes new Cypress tests for each chart that was added.

jenny-s51 requested review from dlabrecq and tlabaj June 25, 2019 20:16

patternfly-build commented Jun 25, 2019

Copy link
Copy Markdown
Collaborator

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

dlabrecq suggested changes Jun 26, 2019
View reviewed changes
Comment thread ...ternfly-4/react-integration/demo-app-ts/src/components/demos/AccordionDemo/AccordionDemo.tsx
Comment thread ...tternfly-4/react-integration/demo-app-ts/src/components/demos/AreaChart/CustomColorsDemo.tsx Outdated
Comment thread ...fly-4/react-integration/demo-app-ts/src/components/demos/BarChartDemo/BlueBarGroupedDemo.tsx Outdated
Comment thread ...rnfly-4/react-integration/demo-app-ts/src/components/demos/BarChartDemo/ColorBarZoomDemo.tsx Outdated
Comment thread ...integration/demo-app-ts/src/components/demos/DonutChartDemo/DonutBottomAlignedLegendDemo.tsx Outdated
Comment thread ...emo-app-ts/src/components/demos/DonutUtilizationChartDemo/DonutUtilizationSimpleLeftDemo.tsx Outdated
Comment thread packages/patternfly-4/react-integration/cypress/integration/areachartcolor.spec.ts
Comment thread ...ternfly-4/react-integration/demo-app-ts/src/components/demos/AccordionDemo/AccordionDemo.tsx
tlabaj requested a review from jcaianirh June 26, 2019 19:14
tlabaj assigned dlabaj and unassigned jcaianirh Jun 26, 2019
tlabaj requested review from dlabaj and removed request for jcaianirh June 26, 2019 19:15
jenny-s51 assigned jenny-s51 and unassigned dlabaj Jun 26, 2019
jenny-s51 requested a review from dlabrecq June 27, 2019 18:42
jenny-s51 requested a review from dlabrecq July 1, 2019 18:09

dlabrecq commented Jul 1, 2019

Copy link
Copy Markdown
Member

Other than the two comments above, the other charts look good to me.

jenny-s51 commented Jul 1, 2019

Copy link
Copy Markdown
Contributor Author

@dlabrecq Thank you! I will fix those right away and push again 🙂

jenny-s51 requested a review from dlabrecq July 1, 2019 19:03
dlabrecq previously approved these changes Jul 1, 2019
View reviewed changes

dlabrecq left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Looks good to me. Nice job! 💯

redallen previously approved these changes Jul 1, 2019
View reviewed changes

redallen left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Thanks for the added tests!

Comment thread packages/patternfly-4/react-core/src/components/Accordion/Accordion.tsx
}: AccordionProps) => (
<dl className={css(styles.accordion, className)} aria-label={ariaLabel} {...props}>
<AccordionContext.Provider value={headingLevel}>{children}</AccordionContext.Provider>
{children}

redallen Jul 1, 2019

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Great catch, this should have never made it in...

jenny-s51 Jul 1, 2019

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Thank you!

jenny-s51 dismissed stale reviews from redallen and dlabrecq via 4c47546 July 1, 2019 19:30
jenny-s51 requested review from dlabrecq and redallen July 1, 2019 19:33
redallen previously approved these changes Jul 1, 2019
View reviewed changes

dlabrecq commented Jul 3, 2019

Copy link
Copy Markdown
Member

After you rebase, you may find that the charts look odd. For example, axis labels and ticks may be missing, charts are larger than normal, etc. This is due to a last minute change in how the Chart component calculates its default padding.

To fix the charts, you'll need to add some padding. The PR below fixes the issue for the chart examples.
https://github.com/patternfly/patternfly-react/pull/2438/files

jenny-s51 added 14 commits July 3, 2019 09:00
dlabaj merged commit 1912733 into patternfly:master Jul 3, 2019
jenny-s51 deleted the iss2231 branch July 3, 2019 15:17
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

dlabaj dlabaj approved these changes
tlabaj Awaiting requested review from tlabaj
+2 more reviewers
dlabrecq dlabrecq approved these changes
redallen redallen left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

dlabaj

jenny-s51

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Add Charts to react-integration

7 participants

Back | FazBrowse Home | New Git URL