Skip to content

Conversation

ericsoderberghp
Copy link
Contributor

@ericsoderberghp ericsoderberghp commented Oct 9, 2022

What does this PR do?

Changed DataChart to better match legend style to chart style.

Previously, opacity wasn't preserved, as can be seen with the legend here:
Screen Shot 2022-10-08 at 8 36 43 PM

Now, the legend more accurately aligns to the styling of each property.
Screen Shot 2022-10-08 at 8 51 16 PM

This was done by refactoring how the seriesStyles are built. That is the essence of these changes. Each chart can set styling, like color and opacity, but also each property can have its own styling for chart types like stacked "bars", "lines", and "areas". In the refactored code, setPropertyStyle() is now used across the various permutations and it is passed an object that spreads the chart styles first and then lets any per-property styles supersede them.

Where should the reviewer start?

DataChart.js where seriesStyles are built, about line 324.

What testing has been done on this PR?

Enhanced the Legend and Stacked Bars stories.

How should this be manually tested?

DataChart stories

Do Jest tests follow these best practices?

no changes to tests, no unit tests were broken in the changing of this code ;)

Do the grommet docs need to be updated?

no

Should this PR be mentioned in the release notes?

yes

Is this change backwards compatible or is it a breaking change?

backwards compatible

Copy link
Collaborator

@jcfilben jcfilben left a comment

Choose a reason for hiding this comment

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

Looks good!

@ericsoderberghp ericsoderberghp merged commit 1f95457 into master Oct 11, 2022
@ericsoderberghp ericsoderberghp deleted the fix/datachart-legend branch October 11, 2022 02:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants