Skip to content

Conversation

@krkshitij
Copy link
Contributor

@krkshitij krkshitij commented Sep 22, 2022

Current Behavior

Narrator announces highlighted chart element label for unhighlighted chart elements when there is a selected legend

New Behavior

Narrator announces correct label for focused chart element when there is a selected legend

@fabricteam
Copy link
Collaborator

fabricteam commented Sep 22, 2022

📊 Bundle size report

🤖 This report was generated against a1c97799460396fc90984176eeb4eb65851062eb

@codesandbox-ci
Copy link

codesandbox-ci bot commented Sep 22, 2022

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 0d31915:

Sandbox Source
@fluentui/react 8 starter Configuration
@fluentui/react-components 9 starter Configuration

@size-auditor
Copy link

size-auditor bot commented Sep 22, 2022

Asset size changes

Size Auditor did not detect a change in bundle size for any component!

Baseline commit: a1c97799460396fc90984176eeb4eb65851062eb (build)

@krkshitij krkshitij marked this pull request as ready for review October 17, 2022 05:30
@krkshitij krkshitij requested a review from a team as a code owner October 17, 2022 05:30
@krkshitij krkshitij requested a review from AtishayMsft October 18, 2022 12:24

private _hoverOn(data: IChartDataPoint, mouseEvent: React.MouseEvent<SVGPathElement>): void {
mouseEvent.persist();
if (this.props.activeArc === this.props.data!.data.legend || this.props.activeArc === '') {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why did you remove these checks

Copy link
Contributor Author

Choose a reason for hiding this comment

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

These checks were not completely removed but used in the setState function call to update the visibility of the callout


private _onBarFocus(pointData: number, color: string, point: IChartDataPoint): void {
if (
this.state.isLegendSelected === false ||
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you explain how setting the state everytime helps.

Copy link
Contributor Author

@krkshitij krkshitij Oct 21, 2022

Choose a reason for hiding this comment

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

Consider this case:

image

Before the change:
When the 3rd (highlighted) bar is focused, the above condition evaluates to true. The setState function is called and the callout becomes visible with details of the 3rd bar. Now when the 4th (unhighlighted) bar is focused, the condition evaluates to false. The setState function is not called and the callout stays visible with details of the previously focused 3rd bar. So the callout's outdated inner text is used for narration.

After the change:
The setState function is called every time a bar is focused. The callout becomes visible when a highlighted bar is focused and hides (but stays in the DOM) when an unhighlighted bar is focused. The callout is always updated with details of the focused bar, which results in correct narration.

@krkshitij krkshitij requested a review from AtishayMsft October 25, 2022 06:14
@krkshitij krkshitij merged commit fd0e28e into microsoft:master Oct 25, 2022
marcosmoura added a commit to marcosmoura/fluentui that referenced this pull request Oct 25, 2022
* master: (106 commits)
  fix: PopoverTriggerChildProps should be exported (microsoft#25159)
  feat: replace ToolbarRadio implementation by usage of toggle button as Radio (microsoft#25343)
  docs: improve Toolbar docs examples (microsoft#25269)
  feat(tools): add unstable API setup updates (microsoft#25355)
  applying package updates
  Fix wrong narration when legend selected (microsoft#24903)
  applying package updates
  chore(react-persona): Update beachball settings and change file's type (microsoft#25363)
  chore: Refactor Field VR tests to have individual tests per component (microsoft#25263)
  chore(react-persona, react-components, vr-tests-v9): Reverting react-persona's version to beta   (microsoft#25357)
  Publishing migration package (microsoft#25354)
  fix: Detailslist is still tabbable when isHeaderVisible=false (microsoft#25342)
  fix: list even/odd off-by-one issue (microsoft#25358)
  feat: add Dropdown a11y spec (microsoft#24917)
  spinbutton: update internal padding for small size (microsoft#25286)
  chore(global-context): migrate to new package structure (microsoft#25341)
  feat: Add validationState to Progress, to make the bar red or green (microsoft#25253)
  feat: Add accessibility scenarios for Fluent UI v9 components #3 (microsoft#23334)
  feat(Dropdown): Freeform search should be case insensitive (microsoft#24879)
  feat(what-input): Limit keyboard detection in inputs (microsoft#25087)
  ...
NotWoods pushed a commit to NotWoods/fluentui that referenced this pull request Nov 18, 2022
* fix wrong narration when legend selected

* add change file

* show/hide callout instead of mount/unmount

* fill transparent inside focusRing

* minor

* add comments
@krkshitij krkshitij deleted the bug-wrong-narration branch February 8, 2023 10:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants