Skip to content

Conversation

@jperals
Copy link
Member

@jperals jperals commented Dec 2, 2022

…ts at a given X coordinate

Description

Similar to PR 538 with LineChart, this PR adds a focused state to AreaChart where all points from all series at a given X coordinate can be highlighted with the keyboard:

Screenshot 2022-12-02 at 15 18 17

The specific series can then be highlighted by pressing the Up and Down keys. At the end of the array we cycle back to highlighting all points.

Related ticket: AWSUI-19257

How has this been tested?

  • Added integration tests and adapted some as necessary to the new interaction
  • Added unit tests
  • Manually tested
  • Manually verified new aria-label with VoiceOver
Review checklist

The following items are to be evaluated by the author(s) and the reviewer(s).

Correctness

  • Changes include appropriate documentation updates.
  • Changes are backward-compatible if not indicated, see CONTRIBUTING.md.
  • Changes do not include unsupported browser features, see CONTRIBUTING.md.
  • Changes were manually tested for accessibility, see accessibility guidelines.

Security

Testing

  • Changes are covered with new/existing unit tests?
  • Changes are covered with new/existing integration tests?

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@codecov
Copy link

codecov bot commented Dec 2, 2022

Codecov Report

Base: 92.36% // Head: 92.49% // Increases project coverage by +0.12% 🎉

Coverage data is based on head (ecf4c01) compared to base (4323c25).
Patch coverage: 97.29% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #547      +/-   ##
==========================================
+ Coverage   92.36%   92.49%   +0.12%     
==========================================
  Files         571      571              
  Lines       16276    16334      +58     
  Branches     4466     4480      +14     
==========================================
+ Hits        15034    15108      +74     
+ Misses       1159     1145      -14     
+ Partials       83       81       -2     
Impacted Files Coverage Δ
src/area-chart/model/interactions-store.ts 83.33% <ø> (+6.19%) ⬆️
...c/internal/components/chart-plot/focus-outline.tsx 96.15% <ø> (ø)
src/internal/components/chart-plot/index.tsx 100.00% <ø> (ø)
src/area-chart/model/use-chart-model.ts 75.65% <97.05%> (+20.30%) ⬆️
src/area-chart/chart-container.tsx 100.00% <100.00%> (ø)
src/internal/styles/colors.ts 100.00% <0.00%> (ø)
src/area-chart/model/compute-chart-props.ts 100.00% <0.00%> (ø)
src/area-chart/model/create-series-decorator.ts 100.00% <0.00%> (ø)
src/internal/components/cartesian-chart/ticks.ts 100.00% <0.00%> (ø)
src/internal/utils/create-category-color-scale.ts 100.00% <0.00%> (ø)
... and 3 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@jperals jperals marked this pull request as ready for review December 6, 2022 07:44
@jperals jperals requested a review from a team as a code owner December 6, 2022 07:44
@jperals jperals requested review from rubencarvalho and removed request for a team December 6, 2022 07:44
rubencarvalho
rubencarvalho previously approved these changes Dec 7, 2022
Copy link
Contributor

@rubencarvalho rubencarvalho left a comment

Choose a reason for hiding this comment

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

LGTM 👍 - just left a comment on one of the tests.

@jperals
Copy link
Member Author

jperals commented Dec 8, 2022

As per discussion with @avinashbot, removed the aria-label that I had added on the VerticalMarker, same as for the Line Chart, see #538 (comment)

},
setupFilesAfterEnv: [path.join(__dirname, 'build-tools', 'jest', 'setup.js')],
testRegex: '(/__tests__/.*(\\.|/)test)\\.[jt]sx?$',
moduleFileExtensions: ['js', 'jsx', 'ts', 'tsx', 'json', 'd.ts'],
Copy link
Member Author

Choose a reason for hiding this comment

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

This is necessary in order to test useChartModel, because it has indirect dependencies on *.d.ts files, which Jest would not be able to load otherwise.

@jperals jperals merged commit fe9e4c3 into main Dec 8, 2022
@jperals jperals deleted the fix/area-chart-keyboard-navigation branch December 8, 2022 14:44
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