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.

Sign up for GitHub

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

Jump to bottom

Fixed: Reverse focus order is not proper in the line chart. #19708

Merged
merged 2 commits into from
Sep 9, 2021
Merged

Fixed: Reverse focus order is not proper in the line chart. #19708

merged 2 commits into from
Sep 9, 2021

Conversation

scharde
Copy link
Contributor

@scharde scharde commented Sep 9, 2021

Pull request checklist

  • Addresses an existing issue: Fixes #0000
  • Include a change request file using $ yarn change

Description of changes

In the line chart, Reverse focus order from legends will work with these changes.
Previously line dots were not rendered for non-active lines, which was removing the line chart tabIndex value. This causes the issue.

With new changes, line dots for all lines will render. If the line is selected then Opacity will be 1 for the line dots, and for non-active lines dots, it is 0.01. By this change, the focus indicator will be there. With this opacity change, the visual appearance is the same as previous and the reverse focus is also working as expected.

Focus areas to test

Line chart

Before Changes:
https://user-images.githubusercontent.com/29042635/132668438-a802db51-e31f-4854-8347-29967a98e058.mp4

After Changes:
https://user-images.githubusercontent.com/29042635/132668852-29cb2049-cdd8-4c74-8538-f22ee2b89542.mp4

Sagar Charde added 2 commits September 9, 2021 15:00
Dots for all points will render with opacity as 1 or 0.01 depends on …
abddf7c 
…line is active or not. This is done becouse reverse focus was not proper in line chart
Change files
56a3562
@scharde scharde mentioned this pull request Sep 9, 2021
Merged
@fabricteam
Copy link
Collaborator

📊 Bundle size report

🤖 This report was generated against 4cb81f4519e08ce941da4e57dd2c55e58c1dddcb

@codesandbox-ci
Copy link

codesandbox-ci bot commented Sep 9, 2021

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 56a3562:

Sandbox Source
Fluent UI React Starter Configuration

@size-auditor
Copy link

size-auditor bot commented Sep 9, 2021

Asset size changes

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

Baseline commit: 4cb81f4519e08ce941da4e57dd2c55e58c1dddcb (build)

@fabricteam
Copy link
Collaborator

Perf Analysis (@fluentui/react)

No significant results to display.

All results

Scenario Render type Master Ticks PR Ticks Iterations Status
Avatar mount 968 926 5000
BaseButton mount 939 959 5000
Breadcrumb mount 2596 2579 1000
ButtonNext mount 481 457 5000
Checkbox mount 1659 1686 5000
CheckboxBase mount 1419 1454 5000
ChoiceGroup mount 5087 5105 5000
ComboBox mount 997 1040 1000
CommandBar mount 10812 10646 1000
ContextualMenu mount 6873 6568 1000
DefaultButton mount 1194 1323 5000
DetailsRow mount 3914 3891 5000
DetailsRowFast mount 3933 3910 5000
DetailsRowNoStyles mount 3773 4083 5000
Dialog mount 2597 2628 1000
DocumentCardTitle mount 153 142 1000
Dropdown mount 3447 3437 5000
FluentProviderNext mount 7287 7350 5000
FluentProviderWithTheme mount 358 340 10
FluentProviderWithTheme virtual-rerender 107 92 10
FluentProviderWithTheme virtual-rerender-with-unmount 458 465 10
FocusTrapZone mount 1835 1840 5000
FocusZone mount 1874 1810 5000
IconButton mount 1860 1928 5000
Label mount 353 356 5000
Layer mount 3158 3126 5000
Link mount 487 484 5000
MakeStyles mount 1775 1826 50000
MenuButton mount 1552 1525 5000
MessageBar mount 2021 2044 5000
Nav mount 3573 3474 1000
OverflowSet mount 1155 1162 5000
Panel mount 2423 2421 1000
Persona mount 895 863 1000
Pivot mount 1503 1517 1000
PrimaryButton mount 1381 1390 5000
Rating mount 8488 8346 5000
SearchBox mount 1491 1462 5000
Shimmer mount 2751 2721 5000
Slider mount 2073 2075 5000
SpinButton mount 5237 5267 5000
Spinner mount 434 434 5000
SplitButton mount 3315 3405 5000
Stack mount 525 563 5000
StackWithIntrinsicChildren mount 1671 1685 5000
StackWithTextChildren mount 5093 4965 5000
SwatchColorPicker mount 10704 10887 5000
Tabs mount 1467 1451 1000
TagPicker mount 2833 2862 5000
TeachingBubble mount 13468 13775 5000
Text mount 437 433 5000
TextField mount 1565 1458 5000
ThemeProvider mount 1237 1295 5000
ThemeProvider virtual-rerender 600 604 5000
ThemeProvider virtual-rerender-with-unmount 2049 2012 5000
Toggle mount 843 851 5000
buttonNative mount 119 125 5000

Perf Analysis (@fluentui/react-northstar)

Perf tests with no regressions
Scenario Current PR Ticks Baseline Ticks Ratio
LayoutMinimalPerf.default 414 380 1.09:1
StatusMinimalPerf.default 763 700 1.09:1
RadioGroupMinimalPerf.default 512 479 1.07:1
CustomToolbarPrototype.default 4185 3899 1.07:1
ImageMinimalPerf.default 422 400 1.06:1
SegmentMinimalPerf.default 384 362 1.06:1
BoxMinimalPerf.default 394 375 1.05:1
GridMinimalPerf.default 366 348 1.05:1
HeaderMinimalPerf.default 393 376 1.05:1
ChatMinimalPerf.default 727 698 1.04:1
ListCommonPerf.default 704 676 1.04:1
SkeletonMinimalPerf.default 380 364 1.04:1
TextAreaMinimalPerf.default 571 547 1.04:1
AnimationMinimalPerf.default 446 435 1.03:1
DropdownManyItemsPerf.default 750 726 1.03:1
DropdownMinimalPerf.default 3218 3124 1.03:1
FormMinimalPerf.default 451 436 1.03:1
LabelMinimalPerf.default 425 414 1.03:1
ProviderMergeThemesPerf.default 1695 1643 1.03:1
TableManyItemsPerf.default 2139 2079 1.03:1
TableMinimalPerf.default 449 437 1.03:1
ToolbarMinimalPerf.default 1003 976 1.03:1
TreeMinimalPerf.default 836 814 1.03:1
ButtonSlotsPerf.default 587 577 1.02:1
CarouselMinimalPerf.default 502 494 1.02:1
CheckboxMinimalPerf.default 2884 2815 1.02:1
RefMinimalPerf.default 234 230 1.02:1
VideoMinimalPerf.default 665 654 1.02:1
AttachmentSlotsPerf.default 1156 1142 1.01:1
ChatDuplicateMessagesPerf.default 316 313 1.01:1
FlexMinimalPerf.default 295 292 1.01:1
ItemLayoutMinimalPerf.default 1281 1271 1.01:1
ListNestedPerf.default 592 586 1.01:1
PortalMinimalPerf.default 176 174 1.01:1
AttachmentMinimalPerf.default 169 169 1:1
ButtonOverridesMissPerf.default 1790 1782 1:1
CardMinimalPerf.default 608 607 1:1
DialogMinimalPerf.default 794 794 1:1
HeaderSlotsPerf.default 824 826 1:1
InputMinimalPerf.default 1284 1282 1:1
ListMinimalPerf.default 544 546 1:1
MenuButtonMinimalPerf.default 1739 1740 1:1
PopupMinimalPerf.default 588 588 1:1
SplitButtonMinimalPerf.default 4378 4390 1:1
TooltipMinimalPerf.default 1059 1057 1:1
AvatarMinimalPerf.default 209 211 0.99:1
DatepickerMinimalPerf.default 5734 5786 0.99:1
EmbedMinimalPerf.default 4344 4371 0.99:1
MenuMinimalPerf.default 881 890 0.99:1
ReactionMinimalPerf.default 406 411 0.99:1
SliderMinimalPerf.default 1592 1605 0.99:1
TextMinimalPerf.default 400 403 0.99:1
DividerMinimalPerf.default 406 416 0.98:1
ListWith60ListItems.default 686 697 0.98:1
TreeWith60ListItems.default 177 180 0.98:1
AccordionMinimalPerf.default 162 169 0.96:1
ButtonMinimalPerf.default 182 189 0.96:1
LoaderMinimalPerf.default 744 775 0.96:1
RosterPerf.default 1232 1284 0.96:1
AlertMinimalPerf.default 275 288 0.95:1
ChatWithPopoverPerf.default 387 408 0.95:1
IconMinimalPerf.default 647 684 0.95:1
ProviderMinimalPerf.default 1030 1110 0.93:1

@Raghurk Raghurk merged commit f333709 into microsoft:master Sep 9, 2021
Raghurk pushed a commit that referenced this pull request Sep 9, 2021
@scharde
Charting: CherryPick #19708: Fixed: Reverse focus order is not proper…
8e0931d 
… in the line chart. #19708 (#19709)

* Dots for all points will render with opacity as 1 or 0.01 depends on line is
active or not. This is done becouse reverse focus was not proper in line chart

* Change files

Co-authored-by: Sagar Charde <v-scharde@microsoft.com>
@msft-fluent-ui-bot
Copy link
Collaborator

🎉@fluentui/react-charting@v5.3.38 has been released which incorporates this pull request.:tada:

Handy links:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Raghurk Raghurk Raghurk approved these changes

Assignees

@GeoffCoxMSFT GeoffCoxMSFT

Labels
Package: charting
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@scharde @fabricteam @msft-fluent-ui-bot @Raghurk @GeoffCoxMSFT