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

Charts: Interactive legend example #3253

Open
wants to merge 1 commit into
base: master
from

Conversation

@dlabrecq
Copy link
Member

dlabrecq commented Nov 1, 2019

@dlabrecq dlabrecq force-pushed the dlabrecq:2352-interactive-legend branch from f036983 to 23bef9f Nov 1, 2019
@codecov-io

This comment has been minimized.

Copy link

codecov-io commented Nov 1, 2019

Codecov Report

Merging #3253 into master will decrease coverage by 0.11%.
The diff coverage is 12%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3253      +/-   ##
==========================================
- Coverage   67.44%   67.33%   -0.12%     
==========================================
  Files         892      893       +1     
  Lines       24867    24917      +50     
  Branches     2141     2150       +9     
==========================================
+ Hits        16772    16778       +6     
- Misses       7091     7135      +44     
  Partials     1004     1004
Flag Coverage Δ
#misc 95.45% <ø> (ø) ⬆️
#patternfly3 69.3% <ø> (ø) ⬆️
#patternfly4 64.55% <12%> (-0.23%) ⬇️
Impacted Files Coverage Δ
...ct-charts/src/components/ChartPoint/ChartPoint.tsx 17.85% <ø> (ø) ⬆️
...t-charts/src/components/ChartPoint/path-helpers.ts 3.5% <0%> (-0.34%) ⬇️
...-4/react-charts/src/components/ChartUtils/index.ts 100% <100%> (ø) ⬆️
.../components/ChartUtils/chart-interactive-legend.ts 11.36% <11.36%> (ø)

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 a9c1bbc...aade734. Read the comment docs.

@dlabrecq dlabrecq force-pushed the dlabrecq:2352-interactive-legend branch from 23bef9f to 06ba2e7 Nov 1, 2019
@patternfly patternfly deleted a comment from patternfly-build Nov 1, 2019
@tlabaj tlabaj requested a review from mturley Nov 5, 2019
@dlabrecq dlabrecq force-pushed the dlabrecq:2352-interactive-legend branch from 06ba2e7 to 683fcb8 Nov 5, 2019
@patternfly-build

This comment has been minimized.

Copy link
Contributor

patternfly-build commented Nov 5, 2019

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

@patternfly patternfly deleted a comment from patternfly-build Nov 5, 2019
@tlabaj tlabaj requested a review from dlabaj Nov 5, 2019
@@ -475,7 +407,7 @@ class InteractiveLegendChart extends React.Component {
}
// Tips:
// 1. Don't omit components unless using custom colors -- that will reassign color scale

This comment has been minimized.

Copy link
@mturley

mturley Nov 6, 2019

Collaborator

Should we move these Tips from the example source code to the Tips section of the markdown documentation below, so they appear without having to expand the code block? I'm not sure, since we do still want people reading this example code to make sure they scroll down to see the tips. If we do move them, maybe we can leave a comment here like // See the Tips section below for important notes about color scale, axis labels and tooltips.

This comment has been minimized.

Copy link
@dlabrecq

dlabrecq Nov 6, 2019

Author Member

Possibly? The tips at the bottom apply to all examples, while this applies only to this particular example. Perhaps we just don't label this as "Tips" and just inline the comments?

This comment has been minimized.

Copy link
@mturley

mturley Nov 7, 2019

Collaborator

Ah, I didn't realize it was specific to this example. I could go either way then.

@mturley
mturley approved these changes Nov 7, 2019
Copy link
Collaborator

mturley left a comment

LGTM

@dlabrecq dlabrecq force-pushed the dlabrecq:2352-interactive-legend branch from 683fcb8 to aade734 Nov 13, 2019
@patternfly-build

This comment has been minimized.

Copy link
Contributor

patternfly-build commented Nov 13, 2019

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

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