Skip to content

Conversation

@behowell
Copy link
Contributor

@behowell behowell commented Feb 15, 2023

Previous Behavior

A previous PR #26815 refactored Input to use makeResetStyles to reduce the number of classes added to Inputs. In it, there were two different reset classes based on whether or not the input was enabled. This resulted in it conditionally calling one of two hooks:

const useRootClassName = disabled ? useRootNonInteractiveClassName : useRootInteractiveClassName;

As pointed out by @spmonahan in #26815 (comment), this technically breaks the rules of hooks by conditionally changing which hooks are called.

New Behavior

Have only a single useRootClassName, which applies the styles for enabled ("interactive") inputs. Merge in all of the interactive styles into their base classes as well.

Update the disabled class to override these hover/focus styles so that the disabled state does not inherit interactive styling.

Related Issue(s)

@behowell behowell self-assigned this Feb 15, 2023
@size-auditor
Copy link

size-auditor bot commented Feb 15, 2023

Asset size changes

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

Baseline commit: 13ccc5915ca86262762f735456f7afe05bd9e537 (build)

@codesandbox-ci
Copy link

codesandbox-ci bot commented Feb 15, 2023

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 00947ba:

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

@fabricteam
Copy link
Collaborator

fabricteam commented Feb 15, 2023

Perf Analysis (@fluentui/react-components)

No significant results to display.

All results

Scenario Render type Master Ticks PR Ticks Iterations Status
Avatar mount 1265 1284 5000
Button mount 920 923 5000
Field mount 1929 1971 5000
FluentProvider mount 1521 1535 5000
FluentProviderWithTheme mount 585 588 10
FluentProviderWithTheme virtual-rerender 545 552 10
FluentProviderWithTheme virtual-rerender-with-unmount 573 574 10
InfoButton mount 511 516 5000
MakeStyles mount 1954 1946 50000
Persona mount 2807 2805 5000
SpinButton mount 2302 2307 5000

@fabricteam
Copy link
Collaborator

fabricteam commented Feb 15, 2023

📊 Bundle size report

Package & Exports Baseline (minified/GZIP) PR Change
react-input
Input
23.89 kB
7.246 kB
22.82 kB
7.163 kB
-1.07 kB
-83 B
react-input
InputField
33.626 kB
10.099 kB
32.699 kB
10.06 kB
-927 B
-39 B
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.979 kB
56.265 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 13ccc5915ca86262762f735456f7afe05bd9e537

@JustSlone
Copy link
Collaborator

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 4 pipeline(s).

@JustSlone
Copy link
Collaborator

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 4 pipeline(s).

@JustSlone
Copy link
Collaborator

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 4 pipeline(s).

@JustSlone
Copy link
Collaborator

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 4 pipeline(s).

@behowell behowell marked this pull request as ready for review February 16, 2023 19:08
@behowell behowell requested review from a team and spmonahan as code owners February 16, 2023 19:08
@behowell behowell closed this Feb 17, 2023
@behowell behowell reopened this Feb 17, 2023
@behowell behowell merged commit fbee0f3 into microsoft:master Feb 21, 2023
@behowell behowell deleted the input/interactive-reset-styles branch February 21, 2023 22:20
marcosmoura added a commit to marcosmoura/fluentui that referenced this pull request Feb 24, 2023
* 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)
  ...
marcosmoura added a commit to marcosmoura/fluentui that referenced this pull request Mar 15, 2023
…r-component

* feat/drawer-base-component: (141 commits)
  remove DrawerContainer
  feat: WIP add initial draft for Drawer and DrawerContainer
  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)
  ...
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.

6 participants