Skip to content

feat: add toolbar controlled state example for toggle buttons#24380

Merged
chpalac merged 17 commits into
microsoft:masterfrom
chpalac:feat/toolbar-controlled-state
Sep 2, 2022
Merged

feat: add toolbar controlled state example for toggle buttons#24380
chpalac merged 17 commits into
microsoft:masterfrom
chpalac:feat/toolbar-controlled-state

Conversation

@chpalac

@chpalac chpalac commented Aug 16, 2022

Copy link
Copy Markdown
Contributor

Overview

Add controlled state feature for ToolbarToggleButton

@fabricteam

fabricteam commented Aug 16, 2022

Copy link
Copy Markdown
Collaborator

📊 Bundle size report

Unchanged fixtures
Package & Exports Size (minified/GZIP)
react-components
react-components: Accordion, Button, FluentProvider, Image, Menu, Popover
188.816 kB
51.903 kB
react-components
react-components: FluentProvider & webLightTheme
33.259 kB
10.952 kB
🤖 This report was generated against 625f79c61c8a496a31d19c615a0b8e0d9a72e55b

@size-auditor

size-auditor Bot commented Aug 16, 2022

Copy link
Copy Markdown

Asset size changes

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

Baseline commit: 625f79c61c8a496a31d19c615a0b8e0d9a72e55b (build)

@fabricteam

fabricteam commented Aug 16, 2022

Copy link
Copy Markdown
Collaborator

Perf Analysis (@fluentui/react-components)

No significant results to display.

All results

Scenario Render type Master Ticks PR Ticks Iterations Status
Avatar mount 1653 1646 5000
Button mount 1142 1204 5000
FluentProvider mount 1944 1925 5000
FluentProviderWithTheme mount 714 715 10
FluentProviderWithTheme virtual-rerender 653 659 10
FluentProviderWithTheme virtual-rerender-with-unmount 712 708 10
MakeStyles mount 2239 2286 50000
SpinButton mount 3315 3147 5000

@ling1726

Copy link
Copy Markdown
Contributor

Was this a bad commit ? You created context values that are never used

@chpalac

chpalac commented Aug 21, 2022

Copy link
Copy Markdown
Contributor Author

Was this a bad commit ? You created context values that are never used

@ling1726 which values? I have created handleToggleButton only

@chpalac
chpalac requested review from a team and khmakoto as code owners August 21, 2022 16:16
@chpalac
chpalac requested a review from ling1726 August 21, 2022 16:16
@chpalac

chpalac commented Aug 21, 2022

Copy link
Copy Markdown
Contributor Author

@ling1726 check if last commit was what you meant.

@codesandbox-ci

codesandbox-ci Bot commented Aug 21, 2022

Copy link
Copy Markdown

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 39dd491:

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

@bsunderhus bsunderhus left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Overall LGTM, I only got one open question: the modification of ToggleButtonState types, they seem to be inconsistent, I might be completely wrong though, but I'm assuming this inconsistency is hard to track due to casting on useToggleState.

Comment thread packages/react-components/react-toolbar/src/components/Toolbar/useToolbar.ts Outdated
Comment thread change/@fluentui-react-button-33f5e569-37ea-4982-a706-573c3a626778.json Outdated
@chpalac
chpalac requested a review from bsunderhus August 31, 2022 13:21
@chpalac
chpalac requested a review from bsunderhus September 1, 2022 12:56
@chpalac
chpalac merged commit bee2c5b into microsoft:master Sep 2, 2022
@chpalac
chpalac deleted the feat/toolbar-controlled-state branch September 2, 2022 12:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants