Skip to content

chore: refactor SpinButton to use makeResetStyles#26867

Merged
spmonahan merged 11 commits intomicrosoft:masterfrom
spmonahan:spinbutton/make-reset-styles
Feb 21, 2023
Merged

chore: refactor SpinButton to use makeResetStyles#26867
spmonahan merged 11 commits intomicrosoft:masterfrom
spmonahan:spinbutton/make-reset-styles

Conversation

@spmonahan
Copy link
Contributor

@spmonahan spmonahan commented Feb 15, 2023

Previous Behavior

SpinButton was implemented before Griffel's makeResetStyles() function was available. SpinButton also called react-input's useStyles() hook as a way to re-use complex styling of the control.

New Behavior

SpinButton uses makeResetStyles to reduce the number of class names present on the control. The dependency on react-input has been removed and the relevant styles are moved into useSpinButtonStyles. This creates code duplication but allows SpinButton to have better control over how its class names are generated.

Class name comparison table

Story Old New
default/controlled/uncontrolled/step 219 😱 16
bounds (when at max bound) 221 😱 26
display value 219 16
size (small) 219 33
appearance (underline) 194 43
appearance (filled lighter) 207 41
appearance (filled darker) 211 32
disabled 187 30

Replaces #26849

@spmonahan spmonahan requested a review from a team as a code owner February 15, 2023 23:17
@size-auditor
Copy link

size-auditor bot commented Feb 15, 2023 •
edited
Loading

Asset size changes

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

Baseline commit: 055d703d6d400235309b038e460d63cadaa3a0c3 (build)

@codesandbox-ci
Copy link

codesandbox-ci bot commented Feb 15, 2023 •
edited
Loading

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 3192f98:

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

@fabricteam
Copy link
Collaborator

fabricteam commented Feb 15, 2023 •
edited
Loading

📊 Bundle size report

Package & Exports Baseline (minified/GZIP) PR Change
react-spinbutton
SpinButton		 44.079 kB
12.356 kB		 32.83 kB
9.864 kB		 -11.249 kB
-2.492 kB
react-spinbutton
SpinButtonField		 53.015 kB
14.806 kB		 41.766 kB
12.254 kB		 -11.249 kB
-2.552 kB
Unchanged fixtures
Package & Exports Size (minified/GZIP)
react-components
react-components: Button, FluentProvider & webLightTheme		 64.09 kB
17.544 kB
react-components
react-components: Accordion, Button, FluentProvider, Image, Menu, Popover		 201.945 kB
56.256 kB
react-components
react-components: FluentProvider & webLightTheme		 35.049 kB
11.533 kB
react-portal-compat
PortalCompatProvider		 6.324 kB
2.129 kB
🤖 This report was generated against 055d703d6d400235309b038e460d63cadaa3a0c3

@fabricteam
Copy link
Collaborator

fabricteam commented Feb 15, 2023 •
edited
Loading

Perf Analysis (@fluentui/react-components)

No significant results to display.

All results

Scenario Render type Master Ticks PR Ticks Iterations Status
Avatar mount 1321 1337 5000
Button mount 947 951 5000
Field mount 2036 2030 5000
FluentProvider mount 1616 1692 5000
FluentProviderWithTheme mount 636 646 10
FluentProviderWithTheme virtual-rerender 592 595 10
FluentProviderWithTheme virtual-rerender-with-unmount 639 631 10
InfoButton mount 568 563 5000
MakeStyles mount 1917 1905 50000
Persona mount 3054 3057 5000
SpinButton mount 2538 2451 5000

@spmonahan spmonahan closed this Feb 16, 2023
@spmonahan spmonahan reopened this Feb 16, 2023
@spmonahan spmonahan closed this Feb 16, 2023
@spmonahan spmonahan reopened this Feb 16, 2023
@JustSlone
Copy link
Collaborator

JustSlone commented Feb 16, 2023

/azp run

@azure-pipelines
Copy link

azure-pipelines bot commented Feb 16, 2023

Azure Pipelines successfully started running 4 pipeline(s).

@JustSlone
Copy link
Collaborator

JustSlone commented Feb 16, 2023

/azp run

@azure-pipelines
Copy link

azure-pipelines bot commented Feb 16, 2023

Azure Pipelines successfully started running 4 pipeline(s).

@JustSlone
Copy link
Collaborator

JustSlone commented Feb 16, 2023

/azp run

@azure-pipelines
Copy link

azure-pipelines bot commented Feb 16, 2023

Azure Pipelines successfully started running 4 pipeline(s).

@JustSlone
Copy link
Collaborator

JustSlone commented Feb 16, 2023

/azp run

@azure-pipelines
Copy link

azure-pipelines bot commented Feb 16, 2023

Azure Pipelines successfully started running 4 pipeline(s).

@JustSlone
Copy link
Collaborator

JustSlone commented Feb 16, 2023

/azp run

@azure-pipelines
Copy link

azure-pipelines bot commented Feb 16, 2023

Azure Pipelines successfully started running 4 pipeline(s).

@JustSlone
Copy link
Collaborator

JustSlone commented Feb 16, 2023

/azp run

@azure-pipelines
Copy link

azure-pipelines bot commented Feb 16, 2023

Azure Pipelines successfully started running 4 pipeline(s).

@spmonahan spmonahan force-pushed the spinbutton/make-reset-styles branch 2 times, most recently from 16412ae to 6105573 Compare February 16, 2023 18:41
behowell
behowell reviewed Feb 16, 2023
View reviewed changes
@spmonahan spmonahan force-pushed the spinbutton/make-reset-styles branch from b8cdf50 to ad7a747 Compare February 16, 2023 23:59
@spmonahan
chore: refactor SpinButton to use makeResetStyles
d2cb140 
- remove dependency on react-input's useStyles hook
- copy styles from input
@spmonahan spmonahan force-pushed the spinbutton/make-reset-styles branch from ad7a747 to 3192f98 Compare February 17, 2023 21:55
@spmonahan spmonahan requested a review from behowell February 17, 2023 22:09
@fabricteam
Copy link
Collaborator

fabricteam commented Feb 17, 2023

🕵 fluentuiv9 No visual regressions between this PR and main

@spmonahan spmonahan merged commit db5fb4b into microsoft:master Feb 21, 2023
marcosmoura added a commit to marcosmoura/fluentui that referenced this pull request Feb 24, 2023
@marcosmoura
Merge branch 'master' into feat/drawer-base-component
f9fb7e8 
* master: (93 commits)
  chore: migrate to jest 27 (microsoft#26835)
  chore: make lint task run without need of build (microsoft#26872)
  chore(react-table): exports UseTableSelectionOptions (microsoft#26892)
  applying package updates
  fix(react-card): allow elements to grow to fill the available space (microsoft#26616)
  fix: Popover without focus trap should not be aria-hidden (microsoft#26932)
  applying package updates
  applying package updates
  fix(react-combobox): Remove _getAriaActiveDescendantValue, compute aria-activedescendantvalue in state, and update currentPendingValue when the options change (microsoft#26574)
  fix: v8 Combobox role and accname for non-hidden icon button (microsoft#26905)
  fix: Removing possible recursive loop in Coachmark (microsoft#26934)
  Combobox: Fix cursor jumping to the end of input (microsoft#26931)
  Fix missing icons on website (microsoft#26797)
  fix: Fix the width of Input's focus border with appearance=underline (microsoft#26881)
  chore: Clean up Input's interactive styles (microsoft#26865)
  Remove codeowners from change files (microsoft#26935)
  chore: add splitbutton error warning to docs, remove button ariaDescription example (microsoft#26904)
  docs: Remove testing code from MenuList example (microsoft#26929)
  chore: refactor SpinButton to use makeResetStyles (microsoft#26867)
  feat: Set overflow on positioned element when `autosize` is applied (microsoft#26868)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@micahgodbolt micahgodbolt micahgodbolt approved these changes

@khmakoto khmakoto khmakoto approved these changes

+1 more reviewer

@behowell behowell behowell approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@micahgodbolt micahgodbolt

Labels

None yet

Projects

None yet

Milestone

February Project Cycle Q1 2023

Development

Successfully merging this pull request may close these issues.

6 participants

@spmonahan @fabricteam @JustSlone @micahgodbolt @khmakoto @behowell