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.

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

SpinButton implementation and styles #22233

Merged
merged 10 commits into from
Apr 5, 2022

Conversation

spmonahan
Copy link
Contributor

Converged SpinButton Implementation

This pull request is the first pass implementation of SpinButton. There is still design work on-going, particularly around the different appearances but the functionality is well-defined and the code in this PR represents a fully working SpinButton. Since I have a working SpinButton I want to get it up for review now. Follow PRs will be made to address missing pieces.

This PR includes:

  1. Fully functional SpinButton control
  2. Storybook stories demonstrating functionality
  3. Passing conformance test
  4. Test coverage for utility functions

This PR does not include:

  1. Complete documentations (do's and don'ts, best practices, etc)
  2. Unit tests for SpinButton control
  3. VR tests for SpinButton control
  4. Migration guide

Relates to #20930

@codesandbox-ci
Copy link

codesandbox-ci bot commented Mar 29, 2022

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 8a00e50:

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

@fabricteam
Copy link
Collaborator

fabricteam commented Mar 29, 2022

📊 Bundle size report

🤖 This report was generated against c77cf524675779b63cf18c193b8c496ad172d530

@size-auditor
Copy link

size-auditor bot commented Mar 29, 2022

Asset size changes

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

Baseline commit: c77cf524675779b63cf18c193b8c496ad172d530 (build)

@fabricteam
Copy link
Collaborator

Perf Analysis (@fluentui/react-components)

No significant results to display.

All results

Scenario Render type Master Ticks PR Ticks Iterations Status
Avatar mount 877 924 5000
Button mount 539 529 5000
FluentProvider mount 1902 1818 5000
FluentProviderWithTheme mount 257 218 10
FluentProviderWithTheme virtual-rerender 214 225 10
FluentProviderWithTheme virtual-rerender-with-unmount 318 301 10
MakeStyles mount 1554 1493 50000

@spmonahan
Copy link
Contributor Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@spmonahan spmonahan force-pushed the spin-button/implementation branch 2 times, most recently from 29180c4 to dfa9e33 Compare March 29, 2022 20:46
@Hotell Hotell removed their assignment Mar 30, 2022
@spmonahan spmonahan changed the title First pass on SpinButton implementation and styles SpinButton implementation and styles Mar 31, 2022
@spmonahan spmonahan self-assigned this Mar 31, 2022
Copy link
Contributor

@smhigley smhigley left a comment

Choose a reason for hiding this comment

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

A few suggestions, but the only one I'm really tied to is handling enter to commit

spmonahan added 10 commits April 4, 2022 12:33
Rework the class name for active buttons to be simpler.
clampWhenInRange() now handles the case when the min value is greater
than the max value. In this case the new value is simply returned with
no clamping applied.

In dev mode an error is written to the console as this is an invalid
state.

Tests have been added to verify the behavior.
Adds support for the Enter key to SpinButton. When focused on the text
input field hitting the enter key will now commit the value.
Size prop was being passed down to the input slot. This was a left over
from attempting to use `Input` rather than `input` for this slot. It is
no longer needed.
@spmonahan spmonahan force-pushed the spin-button/implementation branch from eaa0882 to 8a00e50 Compare April 4, 2022 21:05
@spmonahan spmonahan merged commit 43cb8a8 into microsoft:master Apr 5, 2022
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.

7 participants