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

chore(Video): Convert video to functional component #12799

Merged
merged 11 commits into from
Apr 21, 2020
Merged

chore(Video): Convert video to functional component #12799

merged 11 commits into from
Apr 21, 2020

Conversation

assuncaocharles
Copy link
Contributor

@assuncaocharles assuncaocharles commented Apr 21, 2020 •
edited

Pull request checklist

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

BREAKING CHANGES

Converting Video from class component to functional. Any props are not longer passed to styles function.

Related to #12237

Focus areas to test

(optional)

Microsoft Reviewers: Open in CodeFlow

@msft-github-bot msft-github-bot added the Fluent UI react-northstar (v0) Work related to Fluent UI V0 label Apr 21, 2020
@size-auditor
Copy link

size-auditor bot commented Apr 21, 2020 •
edited

Asset size changes

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

Baseline commit: a88adc3e90f83ae18b9eb5432dceae1d62362a84 (build)

@msft-github-bot
Copy link
Contributor

msft-github-bot commented Apr 21, 2020 •
edited

Perf Analysis

No significant results to display.

All results

Scenario Master Ticks PR Ticks Iterations Status
BaseButton 788 732 5000
Checkbox 1549 1531 5000
CheckboxBase 1222 1255 5000
ChoiceGroup 4589 4801 5000
ComboBox 841 967 1000
CommandBar 6250 6265 1000
ContextualMenu 11181 10975 1000
DefaultButton 1055 950 5000
DetailsRow 3022 3247 5000
DetailsRow (fast icons) 3144 3179 5000
DetailsRow without styles 2846 2753 5000
Dialog 1297 1305 1000
DocumentCardTitle with truncation 1448 1454 1000
Dropdown 2178 2142 5000
FocusZone 1310 1333 5000
IconButton 1577 1490 5000
Label 257 246 5000
Link 409 405 5000
MenuButton 1212 1197 5000
Nav 2751 2763 1000
Panel 1212 1252 1000
Persona 704 727 1000
Pivot 1164 1319 1000
PrimaryButton 1085 1079 5000
SearchBox 1164 1134 5000
Slider 1423 1414 5000
Spinner 342 349 5000
SplitButton 2722 2590 5000
Stack 418 402 5000
Stack with Intrinsic children 933 963 5000
Stack with Text children 3969 3708 5000
TagPicker 2472 2417 5000
Text 313 326 5000
TextField 1210 1257 5000
Toggle 773 767 5000
button 55 53 5000

Perf Analysis (Fluent)

⚠️ 1 potential perf regressions detected

Potential regressions comparing to master

Scenario Current PR Ticks Baseline Ticks Ratio Regression Analysis
VideoMinimalPerf.default 598 752 0.8:1 analysis
Perf comparison
Status Scenario Fluent TPI Fabric TPI Ratio Iterations Ticks
🔧 Avatar.Fluent 0.46 0.43 1.07:1 2000 925
🦄 Button.Fluent 0.1 0.18 0.56:1 5000 524
🔧 Checkbox.Fluent 0.62 0.32 1.94:1 1000 618
🔧 Dialog.Fluent 0.35 0.19 1.84:1 5000 1753
🔧 Dropdown.Fluent 3.13 0.46 6.8:1 1000 3125
🔧 Icon.Fluent 0.14 0.05 2.8:1 5000 675
🎯 Image.Fluent 0.08 0.1 0.8:1 5000 379
🔧 Slider.Fluent 1.24 0.34 3.65:1 1000 1241
🔧 Text.Fluent 0.07 0.02 3.5:1 5000 340
🦄 Tooltip.Fluent 0.08 13.76 0.01:1 5000 401

🔧 Needs work     🎯 On target     🦄 Amazing

Perf tests with no regressions
Scenario Current PR Ticks Baseline Ticks Ratio
ButtonMinimalPerf.default 159 139 1.14:1
ImageMinimalPerf.default 389 353 1.1:1
TableMinimalPerf.default 545 502 1.09:1
FlexMinimalPerf.default 308 285 1.08:1
LabelMinimalPerf.default 450 418 1.08:1
ReactionMinimalPerf.default 1793 1659 1.08:1
SegmentMinimalPerf.default 868 805 1.08:1
AvatarMinimalPerf.default 506 473 1.07:1
ChatWithPopoverPerf.default 522 490 1.07:1
SplitButtonMinimalPerf.default 3534 3312 1.07:1
AttachmentSlotsPerf.default 1103 1039 1.06:1
CardMinimalPerf.default 542 510 1.06:1
PortalMinimalPerf.default 289 272 1.06:1
ToolbarMinimalPerf.default 988 933 1.06:1
Button.Fluent 524 495 1.06:1
LayoutMinimalPerf.default 533 510 1.05:1
MenuMinimalPerf.default 1721 1643 1.05:1
StatusMinimalPerf.default 678 648 1.05:1
TreeWith60ListItems.default 217 206 1.05:1
AnimationMinimalPerf.default 633 609 1.04:1
BoxMinimalPerf.default 329 315 1.04:1
Icon.Fluent 675 649 1.04:1
DialogMinimalPerf.default 1674 1618 1.03:1
DividerMinimalPerf.default 670 651 1.03:1
DropdownMinimalPerf.default 2965 2884 1.03:1
HeaderMinimalPerf.default 471 457 1.03:1
ListCommonPerf.default 934 905 1.03:1
TextAreaMinimalPerf.default 2491 2429 1.03:1
TreeMinimalPerf.default 1265 1232 1.03:1
CheckboxMinimalPerf.default 2729 2685 1.02:1
InputMinimalPerf.default 875 861 1.02:1
ListWith60ListItems.default 1056 1037 1.02:1
Avatar.Fluent 925 905 1.02:1
Dropdown.Fluent 3125 3053 1.02:1
CarouselMinimalPerf.default 554 547 1.01:1
ChatMinimalPerf.default 619 613 1.01:1
FormMinimalPerf.default 705 699 1.01:1
HeaderSlotsPerf.default 1423 1413 1.01:1
LoaderMinimalPerf.default 698 690 1.01:1
RadioGroupMinimalPerf.default 544 540 1.01:1
Checkbox.Fluent 618 612 1.01:1
Image.Fluent 379 375 1.01:1
ListNestedPerf.default 862 861 1:1
MenuButtonMinimalPerf.default 1456 1461 1:1
SliderMinimalPerf.default 1214 1213 1:1
TextMinimalPerf.default 339 340 1:1
Tooltip.Fluent 401 403 1:1
PopupMinimalPerf.default 225 227 0.99:1
ProviderMinimalPerf.default 544 551 0.99:1
RefMinimalPerf.default 174 175 0.99:1
Slider.Fluent 1241 1255 0.99:1
DropdownManyItemsPerf.default 1197 1224 0.98:1
EmbedMinimalPerf.default 3677 3759 0.98:1
ProviderMergeThemesPerf.default 1330 1358 0.98:1
IconMinimalPerf.default 661 672 0.98:1
CustomToolbarPrototype.default 3116 3187 0.98:1
TooltipMinimalPerf.default 707 721 0.98:1
Dialog.Fluent 1753 1782 0.98:1
ChatDuplicateMessagesPerf.default 384 395 0.97:1
HierarchicalTreeMinimalPerf.default 921 954 0.97:1
ListMinimalPerf.default 469 485 0.97:1
AttachmentMinimalPerf.default 138 144 0.96:1
Text.Fluent 340 353 0.96:1
AlertMinimalPerf.default 275 288 0.95:1
GridMinimalPerf.default 613 646 0.95:1
ButtonSlotsPerf.default 501 531 0.94:1
ItemLayoutMinimalPerf.default 1592 1717 0.93:1
AccordionMinimalPerf.default 176 200 0.88:1

@assuncaocharles @layershifter
Update packages/fluentui/react-northstar/src/components/Video/Video.tsx
5a75730 
Co-Authored-By: Oleksandr Fediashov <alexander.mcgarret@gmail.com>
@layershifter
Copy link
Member

Please also added breaking changes to changelog and PR description. Use any previous conversion PR as reference.

@assuncaocharles @layershifter
Update packages/fluentui/react-northstar/src/components/Video/Video.tsx
4ef4294 
Co-Authored-By: Oleksandr Fediashov <alexander.mcgarret@gmail.com>
layershifter
layershifter approved these changes Apr 21, 2020
View reviewed changes
Copy link
Member

@layershifter layershifter left a comment

Choose a reason for hiding this comment

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

Please also update PR description before merge. In this case it will be nothing.
Screenshot_20200421-221008

@assuncaocharles assuncaocharles merged commit 8b14b72 into microsoft:master Apr 21, 2020
@assuncaocharles assuncaocharles deleted the chore/video-functional-component branch April 22, 2020 10:43
DuanShaolong pushed a commit to DuanShaolong/fluentui that referenced this pull request Apr 27, 2020
@assuncaocharles @chpalac
@layershifter @DuanShaolong
chore(Video): Convert video to functional component (microsoft#12799)
4d6cbe6 
* chore(Video): Convert video to functional component

* chore(Video): Fix Test

* Update packages/fluentui/react-northstar/src/components/Video/Video.tsx

Co-Authored-By: Oleksandr Fediashov <alexander.mcgarret@gmail.com>

* chore(Video): Review changes

* chore(Video): Review changes

* chore(Video): add changelog

* Update packages/fluentui/react-northstar/src/components/Video/Video.tsx

Co-Authored-By: Oleksandr Fediashov <alexander.mcgarret@gmail.com>

* chore(Video): VideoStylesProps

* chore(Video): ComponentVariablesInput

* chore(Video): Fix Video Specification Behavior

Co-authored-by: Charles Assuncao <chassunc@microsoft.com>
Co-authored-by: Oleksandr Fediashov <alexander.mcgarret@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@layershifter layershifter layershifter approved these changes

@dvdzkwsk dvdzkwsk Awaiting requested review from dvdzkwsk

@dzearing dzearing Awaiting requested review from dzearing

@jurokapsiar jurokapsiar Awaiting requested review from jurokapsiar

@levithomason levithomason Awaiting requested review from levithomason

@miroslavstastny miroslavstastny Awaiting requested review from miroslavstastny

@mnajdova mnajdova Awaiting requested review from mnajdova

@pompomon pompomon Awaiting requested review from pompomon

Assignees

@joschect joschect

Labels
Fluent UI react-northstar (v0) Work related to Fluent UI V0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@assuncaocharles @msft-github-bot @layershifter @pompomon @joschect @chpalac