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: Refactor Field VR tests to have individual tests per component #25263

Merged
merged 7 commits into from
Oct 24, 2022
Merged

chore: Refactor Field VR tests to have individual tests per component #25263

merged 7 commits into from
Oct 24, 2022

Conversation

behowell
Copy link
Contributor

Current Behavior

The Field VR tests combine all components into a single test, which causes them to be changed any time any component changes. It also makes it harder to identify bugs in a single component.

New Behavior

Refactor Field tests to produce individual stories for each component's tests.

@behowell behowell requested a review from a team October 17, 2022 20:27
@behowell behowell self-assigned this Oct 17, 2022
@behowell behowell requested a review from a team as a code owner October 17, 2022 20:27
@fabricteam
Copy link
Collaborator

fabricteam commented Oct 17, 2022 •
edited
Loading

📊 Bundle size report

🤖 This report was generated against f9436f583d3c291921fe7a4531f69551096e758c

@codesandbox-ci
Copy link

codesandbox-ci bot commented Oct 17, 2022 •
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 6d7ee8c:

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

@size-auditor
Copy link

size-auditor bot commented Oct 17, 2022 •
edited
Loading

Asset size changes

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

Baseline commit: f9436f583d3c291921fe7a4531f69551096e758c (build)

@spmonahan
Copy link
Contributor

Some things I noticed in reviewing Screener. They may be non-issues or issues with the components being wrapped in Field but I wanted to call them out so we can either confirm they are not issues, fix them, or make follow up issues as appropriate.

  1. Progress field doesn't show a warning state
  2. SelectField's Select option overlaps the down chevron. select_field
  3. SliderField is not left aligned (the Slider is too far out to the left). slider_field
  4. SwitchField is not left aligned (the Switch is too far to the right). switch_field

This was referenced Oct 19, 2022
Closed
Closed
@behowell
Copy link
Contributor Author

Thanks @spmonahan! Regarding the issues you found:

  1. The design of Progress doesn't have a warning state. @tomi-msft is following up with design to see if we want to add it. [I'm adding support for the error and success states in a separate PR (feat: Add validationState to Progress, to make the bar red or green #25253), but this one accounts for the lack of warning state.]
  2. That appears to be a bug in the Select component. Logged [Bug] Select's down chevron overlaps with its text #25312 to track it.
  3. That appears to be a bug in the Slider component. Logged [Bug]: Slider's thumb extends past the edges of the control #25313 to track it.
  4. That might be by design. Checkbox and Radio also have a similar indent. But I logged [Bug]: Switch inside Field doesn't align with the Field label #25314 to look into it.

@behowell behowell mentioned this pull request Oct 20, 2022
Closed
2 tasks
@behowell behowell merged commit 68e4386 into microsoft:master Oct 24, 2022
@behowell behowell deleted the field/refactor-vr-tests branch October 24, 2022 22:31
marcosmoura added a commit to marcosmoura/fluentui that referenced this pull request Oct 25, 2022
@marcosmoura
Merge branch 'master' into marcosmoura/feat/selectable-cards
0d8c23f 
* master: (106 commits)
  fix: PopoverTriggerChildProps should be exported (microsoft#25159)
  feat: replace ToolbarRadio implementation by usage of toggle button as Radio (microsoft#25343)
  docs: improve Toolbar docs examples (microsoft#25269)
  feat(tools): add unstable API setup updates (microsoft#25355)
  applying package updates
  Fix wrong narration when legend selected (microsoft#24903)
  applying package updates
  chore(react-persona): Update beachball settings and change file's type (microsoft#25363)
  chore: Refactor Field VR tests to have individual tests per component (microsoft#25263)
  chore(react-persona, react-components, vr-tests-v9): Reverting react-persona's version to beta   (microsoft#25357)
  Publishing migration package (microsoft#25354)
  fix: Detailslist is still tabbable when isHeaderVisible=false (microsoft#25342)
  fix: list even/odd off-by-one issue (microsoft#25358)
  feat: add Dropdown a11y spec (microsoft#24917)
  spinbutton: update internal padding for small size (microsoft#25286)
  chore(global-context): migrate to new package structure (microsoft#25341)
  feat: Add validationState to Progress, to make the bar red or green (microsoft#25253)
  feat: Add accessibility scenarios for Fluent UI v9 components #3 (microsoft#23334)
  feat(Dropdown): Freeform search should be case insensitive (microsoft#24879)
  feat(what-input): Limit keyboard detection in inputs (microsoft#25087)
  ...
NotWoods pushed a commit to NotWoods/fluentui that referenced this pull request Nov 18, 2022
@behowell @NotWoods
chore: Refactor Field VR tests to have individual tests per component (…
854ea4f 
…microsoft#25263)

Refactor Field tests to produce individual stories for each component's tests. The Field VR tests combine all components into a single test, which causes them to be changed any time any component changes. It also makes it harder to identify bugs in a single component.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@spmonahan spmonahan spmonahan approved these changes

Assignees

@behowell behowell

Labels
None yet
Projects
None yet
Milestone
October Project Cycle Q4 2022
Development

Successfully merging this pull request may close these issues.
3 participants
@behowell @fabricteam @spmonahan