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(Alert): Convert Alert class to functional component #12777

Merged
merged 25 commits into from
Apr 20, 2020
Merged

chore(Alert): Convert Alert class to functional component #12777

merged 25 commits into from
Apr 20, 2020

Conversation

assuncaocharles
Copy link
Contributor

@assuncaocharles assuncaocharles commented Apr 19, 2020 •
edited by mnajdova

Pull request checklist

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

Description of changes

This PR converts Alert component to be functional. Restricting props set that will be passed to styles functions.

Related to #12237

Prop sets

Alert
warning
danger
info
success
attached
fitted
dismissible
visible
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 19, 2020
@size-auditor
Copy link

size-auditor bot commented Apr 19, 2020 •
edited

Asset size changes

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

Baseline commit: aedb9d2f99c0ae1b195961c93afbead285c55641 (build)

@msft-github-bot
Copy link
Contributor

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

Perf Analysis

No significant results to display.

All results

Scenario Master Ticks PR Ticks Iterations Status
BaseButton 736 746 5000
Checkbox 1418 1430 5000
CheckboxBase 1166 1164 5000
ChoiceGroup 4575 4517 5000
ComboBox 887 835 1000
CommandBar 7036 6981 1000
ContextualMenu 21499 21614 1000
DefaultButton 971 941 5000
DetailsRow 3063 3161 5000
DetailsRow (fast icons) 3075 3150 5000
DetailsRow without styles 2990 2995 5000
Dialog 1358 1317 1000
DocumentCardTitle with truncation 1525 1542 1000
Dropdown 2231 2138 5000
FocusZone 1498 1489 5000
IconButton 1543 1567 5000
Label 282 280 5000
Link 409 409 5000
MenuButton 1267 1300 5000
Nav 2849 2902 1000
Panel 1352 1332 1000
Persona 766 774 1000
Pivot 1183 1201 1000
PrimaryButton 1093 1077 5000
SearchBox 1136 1126 5000
Slider 1317 1365 5000
Spinner 335 354 5000
SplitButton 2826 2771 5000
Stack 437 431 5000
Stack with Intrinsic children 1061 1026 5000
Stack with Text children 3720 3705 5000
TagPicker 2509 2493 5000
Text 341 330 5000
TextField 1244 1259 5000
Toggle 812 801 5000
button 54 57 5000

Perf Analysis (Fluent)

⚠️ 1 potential perf regressions detected

Potential regressions comparing to master

Scenario Current PR Ticks Baseline Ticks Ratio Regression Analysis
AlertMinimalPerf.default 265 503 0.53:1 analysis
Perf comparison
Status Scenario Fluent TPI Fabric TPI Ratio Iterations Ticks
🔧 Avatar.Fluent 0.45 0.44 1.02:1 2000 903
🦄 Button.Fluent 0.09 0.17 0.53:1 5000 472
🔧 Checkbox.Fluent 0.62 0.3 2.07:1 1000 617
🔧 Dialog.Fluent 0.32 0.19 1.68:1 5000 1616
🔧 Dropdown.Fluent 3.08 0.41 7.51:1 1000 3077
🔧 Icon.Fluent 0.13 0.05 2.6:1 5000 649
🎯 Image.Fluent 0.07 0.1 0.7:1 5000 352
🔧 Slider.Fluent 1.34 0.3 4.47:1 1000 1342
🔧 Text.Fluent 0.07 0.02 3.5:1 5000 331
🦄 Tooltip.Fluent 0.09 17.97 0.01:1 5000 444

🔧 Needs work     🎯 On target     🦄 Amazing

Perf tests with no regressions
Scenario Current PR Ticks Baseline Ticks Ratio
DividerMinimalPerf.default 776 678 1.14:1
PopupMinimalPerf.default 266 237 1.12:1
VideoMinimalPerf.default 759 686 1.11:1
ButtonMinimalPerf.default 148 138 1.07:1
HeaderMinimalPerf.default 466 437 1.07:1
RefMinimalPerf.default 208 197 1.06:1
ChatDuplicateMessagesPerf.default 429 408 1.05:1
MenuButtonMinimalPerf.default 1498 1433 1.05:1
ItemLayoutMinimalPerf.default 1562 1503 1.04:1
LabelMinimalPerf.default 403 388 1.04:1
TextMinimalPerf.default 338 325 1.04:1
AttachmentMinimalPerf.default 148 143 1.03:1
AttachmentSlotsPerf.default 1055 1025 1.03:1
DialogMinimalPerf.default 1652 1602 1.03:1
InputMinimalPerf.default 961 930 1.03:1
ListCommonPerf.default 932 907 1.03:1
SliderMinimalPerf.default 1374 1329 1.03:1
CustomToolbarPrototype.default 3570 3481 1.03:1
Avatar.Fluent 903 877 1.03:1
DropdownManyItemsPerf.default 1310 1283 1.02:1
RadioGroupMinimalPerf.default 535 527 1.02:1
ReactionMinimalPerf.default 1794 1752 1.02:1
ChatMinimalPerf.default 586 579 1.01:1
GridMinimalPerf.default 619 615 1.01:1
HierarchicalTreeMinimalPerf.default 896 891 1.01:1
ListNestedPerf.default 867 857 1.01:1
PortalMinimalPerf.default 279 277 1.01:1
ToolbarMinimalPerf.default 961 950 1.01:1
TooltipMinimalPerf.default 670 666 1.01:1
TreeMinimalPerf.default 1140 1124 1.01:1
DropdownMinimalPerf.default 3087 3088 1:1
EmbedMinimalPerf.default 4060 4075 1:1
FlexMinimalPerf.default 280 280 1:1
MenuMinimalPerf.default 1723 1722 1:1
StatusMinimalPerf.default 657 656 1:1
Checkbox.Fluent 617 615 1:1
Dialog.Fluent 1616 1617 1:1
Dropdown.Fluent 3077 3068 1:1
Tooltip.Fluent 444 446 1:1
ButtonSlotsPerf.default 533 536 0.99:1
ChatWithPopoverPerf.default 549 555 0.99:1
FormMinimalPerf.default 673 677 0.99:1
HeaderSlotsPerf.default 1356 1367 0.99:1
ListMinimalPerf.default 451 456 0.99:1
LoaderMinimalPerf.default 737 745 0.99:1
ProviderMergeThemesPerf.default 1573 1587 0.99:1
SplitButtonMinimalPerf.default 3549 3570 0.99:1
TextAreaMinimalPerf.default 2647 2683 0.99:1
TreeWith60ListItems.default 223 225 0.99:1
Button.Fluent 472 475 0.99:1
AnimationMinimalPerf.default 608 622 0.98:1
CardMinimalPerf.default 506 516 0.98:1
ImageMinimalPerf.default 350 356 0.98:1
ProviderMinimalPerf.default 640 654 0.98:1
TableMinimalPerf.default 518 529 0.98:1
Icon.Fluent 649 665 0.98:1
Image.Fluent 352 361 0.98:1
CarouselMinimalPerf.default 537 551 0.97:1
ListWith60ListItems.default 1074 1102 0.97:1
SegmentMinimalPerf.default 838 865 0.97:1
Slider.Fluent 1342 1381 0.97:1
AccordionMinimalPerf.default 171 178 0.96:1
AvatarMinimalPerf.default 474 492 0.96:1
CheckboxMinimalPerf.default 2722 2849 0.96:1
LayoutMinimalPerf.default 506 527 0.96:1
Text.Fluent 331 349 0.95:1
IconMinimalPerf.default 569 607 0.94:1
BoxMinimalPerf.default 288 312 0.92:1

@assuncaocharles assuncaocharles changed the title [WIP] chore(Alert): Convert to functional component chore(Alert): Convert Alert class to functional component Apr 19, 2020
silviuaavram
silviuaavram reviewed Apr 20, 2020
View reviewed changes
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.

Just a couple of comments

packages/fluentui/react-northstar/src/components/Alert/Alert.tsx Outdated Show resolved Hide resolved
packages/fluentui/react-northstar/src/components/Alert/Alert.tsx Outdated Show resolved Hide resolved
@assuncaocharles @pompomon
Update packages/fluentui/react-northstar/src/components/Alert/Alert.tsx
c7201cc 
Co-Authored-By: Roman Sudarikov <pompomon@users.noreply.github.com>
@assuncaocharles assuncaocharles merged commit ce2b339 into microsoft:master Apr 20, 2020
@assuncaocharles assuncaocharles deleted the chore/alert-functional-comp branch April 23, 2020 14:53
DuanShaolong pushed a commit to DuanShaolong/fluentui that referenced this pull request Apr 27, 2020
@assuncaocharles @chpalac
@layershifter @pompomon @DuanShaolong
chore(Alert): Convert Alert class to functional component (microsoft#…
a339e29 
…12777)

* chore(Alert): Convert to functional component

* chore(Alert): remove import

* chore(Alert): remove dup variable

* chore(Alert): useAutoControlled

* chore(Alert): self controlled props

* chore(Alert): DismissAction as static prop

* chore(Alert): Remove children interface

* chore(Alert): Remove alert state

* chore(Alert): Remove unsed import

* chore(Alert): review changes

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

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

* chore(Alert): remove destruction from selfcontrolled prop

* chore(Alert): remove actionable

* chore(Alert): add Required<>

* chore(Alert): add AlertBehaviorProps

* chore(Alert): Change AlertBehaviorProps import

* chore(Alert): Change AlertBehaviorProps export

* chore(Alert): import useBooleanKnob

* chore(Alert): AlertStylesProps

* chore(Alert): AlertDismissActionStyles

* chore(Alert): Changelog

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

Co-Authored-By: Roman Sudarikov <pompomon@users.noreply.github.com>

* chore(Alert): getA11yprops

* chore(Alert): remove deprecated_className

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

@layershifter layershifter layershifter left review comments

@pompomon pompomon pompomon 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

@silviuaavram silviuaavram Awaiting requested review from silviuaavram

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
7 participants
@assuncaocharles @msft-github-bot @pompomon @silviuaavram @layershifter @joschect @chpalac