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

feat(Card): Selectable card #12921

Merged
merged 18 commits into from
May 19, 2020
Merged

Conversation

pompomon
Copy link
Contributor

@pompomon pompomon commented Apr 29, 2020

Pull request checklist

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

Description of changes

Adding props, styles and examples for selectable cards

Microsoft Reviewers: Open in CodeFlow

@size-auditor
Copy link

size-auditor bot commented Apr 29, 2020

Asset size changes

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

Baseline commit: b66a3351f9b643bc4845fe012e9a3afc42b1654f (build)

@msft-github-bot
Copy link
Contributor

msft-github-bot commented Apr 29, 2020

Perf Analysis

No significant results to display.

All results

Scenario Master Ticks PR Ticks Iterations Status
BaseButton 830 840 5000
Checkbox 1543 1584 5000
CheckboxBase 1347 1323 5000
CheckboxNext 1576 1618 5000
ChoiceGroup 5107 5111 5000
ComboBox 970 902 1000
CommandBar 7336 7236 1000
ContextualMenu 12987 12966 1000
DefaultButton 1091 1093 5000
DetailsRow 3417 3522 5000
DetailsRow (fast icons) 3470 3550 5000
DetailsRow without styles 3192 3263 5000
Dialog 1504 1506 1000
DocumentCardTitle with truncation 1573 1538 1000
Dropdown 2472 2434 5000
FocusZone 1733 1641 5000
IconButton 1748 1728 5000
Label 299 340 5000
Link 448 491 5000
LinkNext 484 476 5000
MenuButton 1448 1414 5000
Nav 3070 3096 1000
Panel 1329 1450 1000
Persona 826 843 1000
Pivot 1267 1287 1000
PrimaryButton 1155 1170 5000
SearchBox 1322 1310 5000
Slider 1456 1526 5000
Spinner 385 370 5000
SplitButton 3011 3019 5000
Stack 477 486 5000
Stack with Intrinsic children 1047 1048 5000
Stack with Text children 4110 4158 5000
TagPicker 2774 2714 5000
Text 396 434 5000
TextField 1396 1383 5000
Toggle 903 842 5000
button 69 66 5000

Perf Analysis (Fluent)

Perf comparison
Status Scenario Fluent TPI Fabric TPI Ratio Iterations Ticks
🔧 Avatar.Fluent 0.47 0.46 1.02:1 2000 944
🦄 Button.Fluent 0.11 0.19 0.58:1 5000 557
🔧 Checkbox.Fluent 0.63 0.35 1.8:1 1000 633
🔧 Dialog.Fluent 0.3 0.2 1.5:1 5000 1514
🔧 Dropdown.Fluent 3.09 0.45 6.87:1 1000 3090
🔧 Icon.Fluent 0.15 0.05 3:1 5000 766
🎯 Image.Fluent 0.08 0.11 0.73:1 5000 396
🔧 Slider.Fluent 1.41 0.34 4.15:1 1000 1408
🔧 Text.Fluent 0.07 0.02 3.5:1 5000 372
🦄 Tooltip.Fluent 0.08 13.97 0.01:1 5000 419

🔧 Needs work     🎯 On target     🦄 Amazing

Perf tests with no regressions
Scenario Current PR Ticks Baseline Ticks Ratio
AttachmentMinimalPerf.default 186 158 1.18:1
TextAreaMinimalPerf.default 537 478 1.12:1
PortalMinimalPerf.default 116 106 1.09:1
ButtonSlotsPerf.default 667 618 1.08:1
CheckboxMinimalPerf.default 3228 2999 1.08:1
ButtonMinimalPerf.default 179 168 1.07:1
GridMinimalPerf.default 797 745 1.07:1
ReactionMinimalPerf.default 388 362 1.07:1
TooltipMinimalPerf.default 752 710 1.06:1
DropdownMinimalPerf.default 3437 3272 1.05:1
ImageMinimalPerf.default 401 381 1.05:1
MenuButtonMinimalPerf.default 1692 1619 1.05:1
ToolbarMinimalPerf.default 907 866 1.05:1
Checkbox.Fluent 633 603 1.05:1
HeaderSlotsPerf.default 1616 1558 1.04:1
Slider.Fluent 1408 1355 1.04:1
BoxMinimalPerf.default 376 364 1.03:1
ChatMinimalPerf.default 685 667 1.03:1
DividerMinimalPerf.default 360 351 1.03:1
FormMinimalPerf.default 475 459 1.03:1
ProviderMinimalPerf.default 641 623 1.03:1
SliderMinimalPerf.default 1325 1288 1.03:1
TreeMinimalPerf.default 1341 1304 1.03:1
AnimationMinimalPerf.default 675 663 1.02:1
EmbedMinimalPerf.default 2095 2044 1.02:1
LabelMinimalPerf.default 429 421 1.02:1
LayoutMinimalPerf.default 611 597 1.02:1
ListNestedPerf.default 941 926 1.02:1
RadioGroupMinimalPerf.default 596 585 1.02:1
IconMinimalPerf.default 692 678 1.02:1
Avatar.Fluent 944 930 1.02:1
Text.Fluent 372 364 1.02:1
CardMinimalPerf.default 609 600 1.01:1
ListCommonPerf.default 1048 1036 1.01:1
LoaderMinimalPerf.default 750 746 1.01:1
SplitButtonMinimalPerf.default 3408 3387 1.01:1
CustomToolbarPrototype.default 3452 3411 1.01:1
ChatDuplicateMessagesPerf.default 436 434 1:1
ItemLayoutMinimalPerf.default 1790 1793 1:1
ProviderMergeThemesPerf.default 1739 1731 1:1
StatusMinimalPerf.default 698 701 1:1
TextMinimalPerf.default 356 357 1:1
Image.Fluent 396 396 1:1
CarouselMinimalPerf.default 510 514 0.99:1
MenuMinimalPerf.default 750 757 0.99:1
SegmentMinimalPerf.default 354 359 0.99:1
ListMinimalPerf.default 500 510 0.98:1
ListWith60ListItems.default 1195 1217 0.98:1
PopupMinimalPerf.default 250 255 0.98:1
RefMinimalPerf.default 195 198 0.98:1
Icon.Fluent 766 778 0.98:1
AlertMinimalPerf.default 318 327 0.97:1
AttachmentSlotsPerf.default 1161 1191 0.97:1
DialogMinimalPerf.default 1565 1607 0.97:1
VideoMinimalPerf.default 642 662 0.97:1
Dialog.Fluent 1514 1562 0.97:1
AvatarMinimalPerf.default 504 525 0.96:1
FlexMinimalPerf.default 313 327 0.96:1
HierarchicalTreeMinimalPerf.default 438 454 0.96:1
InputMinimalPerf.default 964 1007 0.96:1
Dropdown.Fluent 3090 3209 0.96:1
Tooltip.Fluent 419 437 0.96:1
DropdownManyItemsPerf.default 1332 1408 0.95:1
HeaderMinimalPerf.default 518 544 0.95:1
Button.Fluent 557 586 0.95:1
TableMinimalPerf.default 388 411 0.94:1
AccordionMinimalPerf.default 140 151 0.93:1
ChatWithPopoverPerf.default 513 551 0.93:1
TreeWith60ListItems.default 220 242 0.91:1

@pompomon pompomon force-pushed the feat/card-select branch 2 times, most recently from 3fa3f58 to 91b27f5 Compare May 6, 2020 08:08
@pompomon pompomon changed the title [WIP]feat(Card): Selectable card feat(Card): Selectable card May 7, 2020
@kolaps33
Copy link
Contributor

NVDA narrate blank, when navigate in browse mode (after navigation from user card):
Please see logs below:

user card clickable not selected
User #1
blank
out of user card user card clickable not selected
User #2
blank
out of user card user card clickable not selected

@kolaps33
Copy link
Contributor

image

Now reader doesn't narrate position of the card.
I see you keep position of card in aria-label on the element which has role=group
What I think is fine :) only you are missing to reference to the element itself in aria-labelledby.

@kolaps33
Copy link
Contributor

Now we can do selection on the card by "Space" or "Enter" key. General question is, if we want to do it be "Enter" key as well? Just raising the question because in some components only space works for selection. Tree for example. But then in selectable list both Enter and Space work.

I think we will not find any recommendation from ARIA about selectable cards.

Copy link
Contributor

@silviuaavram silviuaavram left a comment

Choose a reason for hiding this comment

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

Only have a couple of comments.

@ecraig12345
Copy link
Member

@pompomon Looks like you need to merge with master. That should also fix the build.

@pompomon
Copy link
Contributor Author

Done :)

@ecraig12345
Copy link
Member

Now it looks like someone needs to accept new screener states. Not doing that myself since I don't actually work on this. :)

Co-authored-by: Oleksandr Fediashov <alexander.mcgarret@gmail.com>
pompomon and others added 2 commits May 19, 2020 10:16
Co-authored-by: Oleksandr Fediashov <alexander.mcgarret@gmail.com>
@msft-github-bot
Copy link
Contributor

Hello @pompomon!

Because this pull request has the AutoMerge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msft-github-bot) and give me an instruction to get started! Learn more here.

@msft-github-bot msft-github-bot merged commit 6cd5e4c into microsoft:master May 19, 2020
@pompomon pompomon deleted the feat/card-select branch May 19, 2020 11:46
miroslavstastny pushed a commit to levithomason/fluentui that referenced this pull request Jun 8, 2020
#### Pull request checklist

- [ ] Addresses an existing issue: Fixes #0000
- [ ] Include a change request file using `$ yarn change`

#### Description of changes

Adding props, styles and examples for selectable cards
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.

None yet

8 participants