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

[Do not review] Enabling selection for input changed for UnifiedPicker and updating example for that case #15412

Closed
wants to merge 6 commits into from

Conversation

nebhatna
Copy link
Contributor

@nebhatna nebhatna commented Oct 7, 2020

Pull request checklist

  • Addresses an existing issue: Fixes #0000
  • Include a change request file using $ yarn change

Description of changes

(give an overview)

Focus areas to test

(optional)

@msft-github-bot
Copy link
Contributor

Thanks for submitting this change, but due to work currently in progress to prepare master for our version 8 beta release, we're asking contributors to either wait a couple weeks to submit changes (if it's not urgent) or submit to the new 7.0 branch (if it's urgent). See #15222 for more details. Thank you for your contribution and understanding!

@codesandbox-ci
Copy link

codesandbox-ci bot commented Oct 7, 2020

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 f58590f:

Sandbox Source
Fluent UI Button Configuration
microsoft/fluentui: codesandbox-react-template Configuration
microsoft/fluentui: codesandbox-react-next-template Configuration
microsoft/fluentui: codesandbox-react-northstar-template Configuration

@size-auditor
Copy link

size-auditor bot commented Oct 7, 2020

Asset size changes

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

Baseline commit: 66ba3997a888bc97e0ec33fc752f3be553d5ccf0 (build)

@@ -270,12 +270,25 @@ export const UnifiedPicker = <T extends {}>(props: IUnifiedPickerProps<T>): JSX.
props.inputProps.onClick(ev as React.MouseEvent<HTMLInputElement>);
}
};
const _onInputChange = (value: string, composing?: boolean) => {
const _onInputChange = (value: string, composing?: boolean, resultItemsList?: T[]) => {
Copy link
Contributor

@elisabethcvs elisabethcvs Oct 8, 2020

Choose a reason for hiding this comment

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

Would this (resultItemList) ever have a value?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, it would, this is what is getting updated

const updatedItems: IPersonaProps[] = [];
const currentItems: IPersonaProps[] = [...peopleSelectedItems];

for (let i = 0; i < currentItems.length; i++) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think you need this, you can have the declaration be this right?
const updatedItems: IPersonaProps[] = [...peopleSelectedItems];

@elisabethcvs
Copy link
Contributor

There's paste logic in the other two examples, should those get updated as well?

@nebhatna nebhatna closed this Oct 20, 2020
@ecraig12345 ecraig12345 deleted the u/nebhatna/fixSelectionInInputChanged branch March 3, 2021 00:01
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.

5 participants