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

new(xychart): add annotations #938

Merged
merged 13 commits into from
Nov 25, 2020
Merged

new(xychart): add annotations #938

merged 13 commits into from
Nov 25, 2020

Conversation

williaster
Copy link
Collaborator

@williaster williaster commented Nov 19, 2020

🚀 Enhancements

This PR integrates the new @visx/annotations into @visx/xychart and updates the demo. Most of the work is wrappers around the annotation components to provide theme + dimensions from XYChart's context, and I also added an AnimatedAnnotation variant so that they can animate upon data changes.

A quick summary of components added for ease of review

  • private/BaseAnnotation powers Annotation and AnimatedAnnotation (mirrors how AnimatedSeries work)
  • Annotation
  • AnimatedAnnotation

Animation

Editable

Theme support

Horizontal line annotations

Noting that you can render an annotation per data point, so this PR fixes #903

Something like this

{data.map((d, i) => (
  <AnimatedAnnotation key={i} dataKey="New York" datum={d} dx={0} dy={-10}>
    <AnnotationLabel
      subtitle={`${d['New York']}°F`}
      horizontalAnchor="middle"
      verticalAnchor="middle"
      showBackground={false}
      showAnchorLine={false}
      width={50}
    />
  </AnimatedAnnotation>
))}

💥 Breaking Changes

This updates the shape of the xychart theme, but since it's not released it's not technically breaking in a semantic version sense.

@kristw @hshoff

@williaster williaster added this to the 1.1.1 milestone Nov 19, 2020
@williaster williaster added this to Reviewable in XYChart via automation Nov 19, 2020
curve={curve}
/>
{!renderBarSeries && (
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

this just makes it so we render 3 lines when we don't render a bar series, ditto on area below

/** Annotation component to render. */
AnnotationComponent: React.FC<AnnotationProps> | React.FC<EditableAnnotationProps>;
/** Key for series to which datum belongs (used for x/yAccessors). Alternatively xAccessor + yAccessor may be specified. */
dataKey: string;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

need to make this optional since we allow you to override x/yAccessors

@@ -42,14 +43,20 @@ type ProvidedProps = {
y: Accessors;
date: Accessor;
};
animationTrajectory: AnimationTrajectory;
annotationDataKey: keyof Accessors | null;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

in this file, just added the annotation state + re-ordered several controls to group them more logically (lots of controls were added over time so it's a little overwhelming)

Copy link
Collaborator

@kristw kristw left a comment

Choose a reason for hiding this comment

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

lgtm

Base automatically changed from chris--annotations-edit-labelsubject to master November 23, 2020 20:06
@williaster williaster modified the milestones: 1.2.0, 1.2.1 Nov 25, 2020
@coveralls
Copy link

Pull Request Test Coverage Report for Build 383875944

  • 9 of 45 (20.0%) changed or added relevant lines in 10 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-1.06%) to 59.864%

Changes Missing Coverage Covered Lines Changed/Added Lines %
packages/visx-xychart/src/components/annotation/Annotation.tsx 0 1 0.0%
packages/visx-xychart/src/components/annotation/AnnotationCircleSubject.tsx 0 2 0.0%
packages/visx-xychart/src/components/annotation/AnnotationConnector.tsx 0 2 0.0%
packages/visx-xychart/src/components/annotation/AnnotationLineSubject.tsx 0 2 0.0%
packages/visx-xychart/src/components/annotation/AnnotationLabel.tsx 1 5 20.0%
packages/visx-xychart/src/components/annotation/AnimatedAnnotation.tsx 0 9 0.0%
packages/visx-xychart/src/components/annotation/private/BaseAnnotation.tsx 1 17 5.88%
Totals Coverage Status
Change from base Build 382529313: -1.06%
Covered Lines: 1634
Relevant Lines: 2571

💛 - Coveralls

@williaster
Copy link
Collaborator Author

will add some tests in another PR to get coverage pack up

@williaster williaster merged commit 1a051a9 into master Nov 25, 2020
XYChart automation moved this from Reviewable to Done Nov 25, 2020
@williaster williaster deleted the chris--xychart-annotations branch November 25, 2020 19:48
@williaster williaster modified the milestones: 1.2.1, 1.3.0 Dec 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
XYChart
  
Done
Development

Successfully merging this pull request may close these issues.

feature(xychart): add ability to label any series point
3 participants