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

Issue 22528 picker required attribute #23362

Conversation

tpalacino
Copy link

Current Behavior

The Picker components that extend the BasePicker and BasePickerListBelow are missing properties that most other FluentUI user input component support. The missing properties include label, required, errorMessage, and onGetErrorMessage.

New Behavior

The pickers now support the following optional properties:

Property Description
label When specified renders consistently with how a label is rendered for a TextField.
required When specified renders consistently with how the required attribute is rendered for a TextField.
errorMessage When specified renders consistently with how an error message is rendered for a ComboBox.
onGetErrorMessage When specified behaves consistently with how an error message function behaves for a TextField.

Related Issue(s)

Fixes #22528

@ghost
Copy link

ghost commented Jun 2, 2022

CLA assistant check
All CLA requirements met.

@codesandbox-ci
Copy link

codesandbox-ci bot commented Jun 2, 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 269d157:

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

@fabricteam
Copy link
Collaborator

fabricteam commented Jun 2, 2022

📊 Bundle size report

🤖 This report was generated against 3bf2de945735d2464b4aa4fcfccbf636a74651ef

@size-auditor
Copy link

size-auditor bot commented Jun 2, 2022

Asset size changes

Project Bundle Baseline Size New Size Difference
office-ui-fabric-react fluentui-react-Pickers 280.499 kB 283.351 kB ExceedsTolerance     2.852 kB

ExceedsTolerance Over Tolerance (1024 B) ExceedsBaseline Over Baseline BelowBaseline Below Baseline New New Deleted  Removed 1 kB = 1000 B

Baseline commit: 36b914d0bbdad6bf5ee88204ec44f3a5a9b47936 (build)

@fabricteam
Copy link
Collaborator

fabricteam commented Jun 2, 2022

Perf Analysis (@fluentui/react)

Scenario Render type Master Ticks PR Ticks Iterations Status
TagPicker mount 1414 23066 5000 Possible regression
All results

Scenario Render type Master Ticks PR Ticks Iterations Status
BaseButton mount 614 621 5000
Breadcrumb mount 1709 1674 1000
Checkbox mount 1672 1695 5000
CheckboxBase mount 1465 1477 5000
ChoiceGroup mount 2963 2880 5000
ComboBox mount 654 643 1000
CommandBar mount 6188 6097 1000
ContextualMenu mount 11711 11848 1000
DefaultButton mount 747 738 5000
DetailsRow mount 2179 2259 5000
DetailsRowFast mount 2170 2124 5000
DetailsRowNoStyles mount 1981 2020 5000
Dialog mount 2742 2786 1000
DocumentCardTitle mount 226 224 1000
Dropdown mount 1937 1980 5000
FocusTrapZone mount 1140 1094 5000
FocusZone mount 1030 1076 5000
GroupedList mount 40784 41053 2
GroupedList virtual-rerender 19655 19749 2
GroupedList virtual-rerender-with-unmount 50009 50215 2
GroupedListV2 mount 229 227 2
GroupedListV2 virtual-rerender 217 207 2
GroupedListV2 virtual-rerender-with-unmount 224 224 2
IconButton mount 1070 1074 5000
Label mount 334 328 5000
Layer mount 2700 2694 5000
Link mount 384 399 5000
MenuButton mount 902 917 5000
MessageBar mount 21326 21369 5000
Nav mount 1942 1927 1000
OverflowSet mount 769 776 5000
Panel mount 1776 1777 1000
Persona mount 727 769 1000
Pivot mount 858 848 1000
PrimaryButton mount 845 828 5000
Rating mount 4517 4584 5000
SearchBox mount 900 922 5000
Shimmer mount 1880 1790 5000
Slider mount 1288 1294 5000
SpinButton mount 2835 2822 5000
Spinner mount 390 376 5000
SplitButton mount 1825 1812 5000
Stack mount 399 409 5000
StackWithIntrinsicChildren mount 845 843 5000
StackWithTextChildren mount 2545 2572 5000
SwatchColorPicker mount 6090 6118 5000
TagPicker mount 1414 23066 5000 Possible regression
Text mount 362 368 5000
TextField mount 909 928 5000
ThemeProvider mount 817 824 5000
ThemeProvider virtual-rerender 601 582 5000
ThemeProvider virtual-rerender-with-unmount 1283 1272 5000
Toggle mount 605 615 5000
buttonNative mount 181 179 5000

@tpalacino
Copy link
Author

tpalacino commented Jun 3, 2022

This process would flow much more smoothly if the Full development workflow walkthrough were not breaking on unchanged files from the master branch of the microsoft/fluentui repo
yarn-buildci-output.txt

@tpalacino
Copy link
Author

tpalacino commented Jun 3, 2022

@ling1726 The "Previously Accepted" view of the "PeoplePicker: List disabled" didn't not look disabled. The "Current" view of the "PeoplePicker: List disabled" actually looks disabled. Please correct me if I'm wrong, but that shouldn't be a blocker, it should resolve a previously unidentified bug

@tpalacino
Copy link
Author

tpalacino commented Jun 3, 2022

@ling1726 Also, please provide guidance on how to address the "SizeAuditor" issue.

@tpalacino
Copy link
Author

Please provide some guidance on how to address the remaining failures.

@ling1726 The "Previously Accepted" view of the "PeoplePicker: List disabled" didn't not look disabled. The "Current" view of the "PeoplePicker: List disabled" actually looks disabled. Please correct me if I'm wrong, but that shouldn't be a blocker, it should resolve a previously unidentified bug

@ling1726 Also, please provide guidance on how to address the "SizeAuditor" issue.

Image showing the previously accepted visual state is invalid as the control does not appear disabled

image

Image showing the current visual state is actually valid as the control appears disabled

image

@tpalacino
Copy link
Author

@fabricteam It looks like merging the latest changes from master a couple days ago fixed the erroneous visual test failure. Can anyone offer any guidance on the Sizer issue. I can't find anything useful in the documentation about how to address that type of issue.

@tpalacino
Copy link
Author

Is there anybody that has contributed to this project before that can help me understand how to proceed?

@tpalacino tpalacino requested a review from a team as a code owner October 22, 2022 18:23
@tpalacino tpalacino requested a review from a team as a code owner April 12, 2023 12:44
@tpalacino
Copy link
Author

@fabricteam Anybody available to take a look at this yet?

@tpalacino
Copy link
Author

Superceeded by PR #33243.

@tpalacino tpalacino closed this Nov 10, 2024
@tpalacino tpalacino deleted the issue-22528-picker-required-attribute branch November 10, 2024 17:13
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.

NormalPeoplePicker required attribute
4 participants