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

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

Merged
merged 14 commits into from Jul 3, 2019

Conversation

@jenny-s51
Copy link
Contributor

jenny-s51 commented Jun 25, 2019

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

@jenny-s51 jenny-s51 requested review from dlabrecq and tlabaj Jun 25, 2019
@patternfly-build

This comment has been minimized.

Copy link
Contributor

patternfly-build commented Jun 25, 2019

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

@tlabaj tlabaj requested a review from jcaianirh Jun 26, 2019
@tlabaj tlabaj assigned dlabaj and unassigned jcaianirh Jun 26, 2019
@tlabaj tlabaj requested review from dlabaj and removed request for jcaianirh Jun 26, 2019
@jenny-s51 jenny-s51 assigned jenny-s51 and unassigned dlabaj Jun 26, 2019
@jenny-s51 jenny-s51 requested a review from dlabrecq Jun 27, 2019
@jenny-s51 jenny-s51 force-pushed the jenny-s51:iss2231 branch from 65b73ed to b3c40b4 Jun 27, 2019
@jenny-s51 jenny-s51 force-pushed the jenny-s51:iss2231 branch from 3ffe72b to 3410a0c Jun 28, 2019
@jenny-s51 jenny-s51 requested a review from dlabrecq Jul 1, 2019
@dlabrecq

This comment has been minimized.

Copy link
Member

dlabrecq commented Jul 1, 2019

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

@jenny-s51

This comment has been minimized.

Copy link
Contributor Author

jenny-s51 commented Jul 1, 2019

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

@jenny-s51 jenny-s51 requested a review from dlabrecq Jul 1, 2019
Copy link
Member

dlabrecq left a comment

Looks good to me. Nice job! 💯

Copy link
Contributor

redallen left a comment

Thanks for the added tests!

@@ -24,6 +24,5 @@ export const Accordion: React.FunctionComponent<AccordionProps> = ({
}: AccordionProps) => (
<dl className={css(styles.accordion, className)} aria-label={ariaLabel} {...props}>
<AccordionContext.Provider value={headingLevel}>{children}</AccordionContext.Provider>
{children}

This comment has been minimized.

Copy link
@redallen

redallen Jul 1, 2019

Contributor

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

This comment has been minimized.

Copy link
@jenny-s51

jenny-s51 Jul 1, 2019

Author Contributor

Thank you!

@jenny-s51 jenny-s51 dismissed stale reviews from redallen and dlabrecq via 4c47546 Jul 1, 2019
@jenny-s51 jenny-s51 requested review from dlabrecq and redallen Jul 1, 2019
@dlabrecq

This comment has been minimized.

Copy link
Member

dlabrecq commented Jul 3, 2019

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 jenny-s51 force-pushed the jenny-s51:iss2231 branch from 4c47546 to 1034d22 Jul 3, 2019
@dlabaj
dlabaj approved these changes Jul 3, 2019
@dlabaj dlabaj merged commit 1912733 into patternfly:master Jul 3, 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
@jenny-s51 jenny-s51 deleted the jenny-s51:iss2231 branch Jul 3, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
7 participants
You can’t perform that action at this time.