fix(Dropdown): don't clear value on same item selection #12784
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
silviuaavram
requested review from
dvdzkwsk,
dzearing,
jurokapsiar,
layershifter,
levithomason,
miroslavstastny,
mnajdova and
pompomon
as code owners
msft-github-bot
added
the
Fluent UI react-northstar (v0)
Asset size changesSize Auditor did not detect a change in bundle size for any component! Baseline commit: 11d5a626aac03900f8e8e8f82379374323c93639 (build) |
Perf AnalysisNo significant results to display. All results
Perf Analysis (Fluent)Perf comparison
Perf tests with no regressions
|
pompomon
reviewed
Apr 22, 2020
| @@ -912,14 +912,18 @@ class Dropdown extends AutoControlledComponent<WithAsProp<DropdownProps>, Dropdo | |||
| case Downshift.stateChangeTypes.keyDownEnter: | |||
| case Downshift.stateChangeTypes.clickItem: | |||
| const shouldAddHighlightedIndex = !multiple && items && items.length > 0; | |||
| const isSameItemSelected = changes.selectedItem === undefined; | |||
There was a problem hiding this comment.
const isSameItemSelected = changes.selectedItem === undefined;
const newValue = isSameItemSelected ? value[0] : changes.selectedItem;
newState.open = false;
newState.searchQuery = this.getSelectedItemAsString(newValue);
if (!isSameItemSelected) {
newState.value = multiple ? [...value, changes.selectedItem] : [changes.selectedItem];
newState.highlightedIndex = shouldAddHighlightedIndex ? items.indexOf(newValue) : null;
if (getA11ySelectionMessage && getA11ySelectionMessage.onAdd) {
this.setA11ySelectionMessage(getA11ySelectionMessage.onAdd(changes.selectedItem));
}
}
nit
Would having newState.value, newState.highlightedIndex and this.setA11ySelectionMessage inside ! isSameItemSelected condition work the same?
There was a problem hiding this comment.
cool! it might work, let me try!
pompomon
approved these changes
Apr 22, 2020
…office-ui-fabric-react into fix/dropdown-same-selection
assuncaocharles
pushed a commit
to assuncaocharles/fluentui
that referenced
this pull request
Apr 23, 2020
assuncaocharles
added a commit
that referenced
this pull request
Apr 24, 2020
* Roster prototype * Section not public. * firt raw version of multiselect tree availability * unit tests fix * changing to proper default selection usage * adressing comment from review, moving Enter filtration to behaviors * improving code * polish name, removing not used import * fixing based on the review, adding screener tests * removing not used line * adding behavior test and running lint fix * feat(Roster): Suggestion with prop `as` * feat(Roster): Fix keyboard navigation * feat(Roster): Add Selecttion event * changing shorthand from ts to tsx * feat(Roster): Add multiselection * feat(Roster): Remove Console * adressing review changes and fix console errors * feat(Roster): Fix styles for select all * fixing unstability in screener toolbar test * improving screener test to cover one more scenario * feat(Roster): Adding custom idicator prop to the tree * feat(Roster): Click on item * feat(Roster): Fix selection * feat(Roster): add data-is-focusable to custom checkbox * feat(Roster): Rename props * fixes from review * fix from merging another branch * map prop to behavior * removing roster prototype * removing leftover from sidebar * changing name for custom checkbox * fixing navigation into cell which has multiple focusable elements * updating changelog * feat(Roster): Replace as with children in the data * feat(Tree): Fix tree selection * feat(Tree): Remove dead file * adding table keyboard navigation tests * deleting from master * adding table keyboard navigation tests * deleting files from master, pushed by mistaked * feat(Tree): remove dup spread props * feat(Tree): Revert context value prop * Update packages/fluentui/accessibility/src/behaviors/Tree/treeItemBehavior.ts Co-Authored-By: Roman Sudarikov <pompomon@users.noreply.github.com> * Update packages/fluentui/react-northstar/src/components/Tree/TreeItem.tsx Co-Authored-By: Marija Najdova <mnajdova@gmail.com> * Update packages/fluentui/react-northstar/src/components/Tree/TreeTitle.tsx Co-Authored-By: Marija Najdova <mnajdova@gmail.com> * Update packages/fluentui/react-northstar/src/themes/teams/components/Tree/treeTitleVariables.ts Co-Authored-By: Marija Najdova <mnajdova@gmail.com> * Update packages/fluentui/accessibility/src/behaviors/Tree/treeTitleBehavior.ts Co-Authored-By: Oleksandr Fediashov <alexander.mcgarret@gmail.com> * Update packages/fluentui/react-northstar/src/components/Tree/Tree.tsx Co-Authored-By: Oleksandr Fediashov <alexander.mcgarret@gmail.com> * Update packages/fluentui/react-northstar/src/components/Tree/TreeItem.tsx Co-Authored-By: Oleksandr Fediashov <alexander.mcgarret@gmail.com> * feat(Tree): Temp removing option to pass a custom checkbox to the selectable tree * Update packages/fluentui/docs/src/examples/components/Tree/Usage/TreeMultiselectExample.shorthand.tsx Co-Authored-By: Oleksandr Fediashov <alexander.mcgarret@gmail.com> * feat(Tree): Reduce over forEach * feat(Tree): Typo * Update packages/fluentui/accessibility/test/behaviors/treeTitleBehavior-test.tsx Co-Authored-By: Roman Sudarikov <pompomon@users.noreply.github.com> * feat(Tree): Enable select all nested children * feat(Tree): Fix select on right click * feat(Tree): Reverting non-related file * feat(Tree): Fix tree title behavior tests * feat(Tree): Fix behavior test for subTree * feat(Tree): Outline treetitle as block when selectable * feat(Tree): Custom Select Indicator * feat(Tree): Remove focus from custom checkbox * feat(Tree): fix select all * removing rtl types and fix if condition for not selectable item * adding example that will handle custom selectable parent and default * adding methods to check if all or some of children of a groups has been selected * handling incompleted group selected * adding icon for 3rd checkbox state * feat(Tree): Improve determinated state for selectable parent * feat(Tree): enable toggle parent select using keyboard * feat(Tree): indeterminated state url file * chore(treeTitleVariables): remove lodash * feat(Tree): border color * feat(Tree): add step * feat(Tree): Remove Dup File * feat(Tree): Right condition * feat(Tree): Adapt example * feat(Tree): Fix behavior test * feat(Tree): Toggle multiselection * feat(Tree): Toggle multiselection * feat(Tree): Remove keyboard import * feat(Tree): Fix conditions * feat(Tree): Fix test behaviors * feat(Tree): Remove select on enter * feat(Tree): Active items test * feat(Tree): Space select screenshot * feat(Tree): Review changes * feat(Tree): Fix test * feat(Tree): remove log * feat(Tree): steps * fix(Dropdown): don't clear value on same item selection (#12784) * fix same item selection * improve the unit test * changelog * improve code * hide/unhide legends (#12469) * adding new prop hideLegend in donut chart and line chart which decides to show or hide legends * Change files Co-authored-by: v-sivsar <v-sivsar@microsoft.com> * User/v sivsar/linchart line click (#12548) * adding onclick to the line in the line chart * Change files Co-authored-by: v-sivsar <v-sivsar@microsoft.com> * chore(TableRow): Convert to FC (#12797) * Convert TableRow to FC * Update Changlelog * Fix imports * Apply suggestions from code review Co-Authored-By: Marija Najdova <mnajdova@gmail.com> * Add TableRow onClick to handled props * Properly add onClick to handledProps Co-authored-by: Marija Najdova <mnajdova@gmail.com> * FocusZone: Adding preventDefaultWhenHandled to v7 version (#12636) #### Pull request checklist - [ ] Addresses an existing issue: Fixes #0000 - [x] Include a change request file using `$ yarn change` #### Description of changes This PR brings the `preventDefaultWhenHandled` prop from the v0 version of `FocusZone` to the v7 version. This prop, if true, prevents the default behavior of focus events. #### Focus areas to test (optional) * FocusZone: Updating comment for preventDefaultWhenHandled prop (#12830) #### Pull request checklist - [ ] Addresses an existing issue: Fixes #0000 - [x] Include a change request file using `$ yarn change` #### Description of changes `FocusZone`: Updating comment for `preventDefaultWhenHandled` prop to be consistent across both versions. #### Focus areas to test (optional) * feature: improve `compose()` signature (#12745) * feat: compose sig * wip * bump api * -cleanup * -changed compose tests * -fixed any * -fixed screener regressions * -fixed build issues -fixed screener * Update packages/react-compose/src/types.ts Co-Authored-By: Oleksandr Fediashov <olfedias@microsoft.com> * fix types in ButtonContent * -update api * -test updates * -ts fixes * -added shorthandConfig in Button component * -fixed compose test * -fix * Update packages/fluentui/react-northstar/src/components/Box/Box.tsx Co-Authored-By: Roman Sudarikov <pompomon@users.noreply.github.com> * Update packages/fluentui/react-northstar/src/components/Button/Button.tsx Co-Authored-By: Roman Sudarikov <pompomon@users.noreply.github.com> * fixes * wip * fix RefForward to handle `innerRef` changes * reverted Accordion changes * -tslint disable * -screener diff * -testing * -reverted changes * -better description * fix build * changelog entry * Update packages/fluentui/CHANGELOG.md Co-Authored-By: Oleksandr Fediashov <olfedias@microsoft.com> Co-authored-by: Marija Najdova <mnajdova@gmail.com> Co-authored-by: Roman Sudarikov <pompomon@users.noreply.github.com> * applying package updates * Callout: Updating positioning logic to account for dual screen scenarios (#12785) #### Pull request checklist - [ ] Addresses an existing issue: Fixes #0000 - [x] Include a change request file using `$ yarn change` #### Description of changes This PR updates the positioning logic in the `Callout` component to make it dual-screen aware using the new `windowSegments` API. Components using `Callout` underneath such as `ContextualMenu` are also updated. This PR also adds a bunch of unit tests checking that the correct bounds for the screen segments are being calculated. __Before:__  __After:__  #### Focus areas to test * feat(Tree): steps Co-authored-by: Michal <michal.frdlik@hotmail.com> Co-authored-by: Milan Turon <mituron@microsoft.com> Co-authored-by: Milan Turon <milanturon@centrum.cz> Co-authored-by: Charles Assuncao <chassunc@microsoft.com> Co-authored-by: Roman Sudarikov <pompomon@users.noreply.github.com> Co-authored-by: Marija Najdova <mnajdova@gmail.com> Co-authored-by: Oleksandr Fediashov <alexander.mcgarret@gmail.com> Co-authored-by: Charles Assuncao <charles.assuncao@microsoft.com> Co-authored-by: Silviu Alexandru Avram <silviu.avram91@gmail.com> Co-authored-by: shivasai09 <33802398+shivasai09@users.noreply.github.com> Co-authored-by: v-sivsar <v-sivsar@microsoft.com> Co-authored-by: Makoto Morimoto <humbertomakotomorimoto@gmail.com> Co-authored-by: Oleksandr Fediashov <olfedias@microsoft.com> Co-authored-by: Fluent UI Build <fluentui-internal@service.microsoft.com>
DuanShaolong
pushed a commit
to DuanShaolong/fluentui
that referenced
this pull request
Apr 27, 2020
DuanShaolong
pushed a commit
to DuanShaolong/fluentui
that referenced
this pull request
Apr 27, 2020
…oft#12822) * Roster prototype * Section not public. * firt raw version of multiselect tree availability * unit tests fix * changing to proper default selection usage * adressing comment from review, moving Enter filtration to behaviors * improving code * polish name, removing not used import * fixing based on the review, adding screener tests * removing not used line * adding behavior test and running lint fix * feat(Roster): Suggestion with prop `as` * feat(Roster): Fix keyboard navigation * feat(Roster): Add Selecttion event * changing shorthand from ts to tsx * feat(Roster): Add multiselection * feat(Roster): Remove Console * adressing review changes and fix console errors * feat(Roster): Fix styles for select all * fixing unstability in screener toolbar test * improving screener test to cover one more scenario * feat(Roster): Adding custom idicator prop to the tree * feat(Roster): Click on item * feat(Roster): Fix selection * feat(Roster): add data-is-focusable to custom checkbox * feat(Roster): Rename props * fixes from review * fix from merging another branch * map prop to behavior * removing roster prototype * removing leftover from sidebar * changing name for custom checkbox * fixing navigation into cell which has multiple focusable elements * updating changelog * feat(Roster): Replace as with children in the data * feat(Tree): Fix tree selection * feat(Tree): Remove dead file * adding table keyboard navigation tests * deleting from master * adding table keyboard navigation tests * deleting files from master, pushed by mistaked * feat(Tree): remove dup spread props * feat(Tree): Revert context value prop * Update packages/fluentui/accessibility/src/behaviors/Tree/treeItemBehavior.ts Co-Authored-By: Roman Sudarikov <pompomon@users.noreply.github.com> * Update packages/fluentui/react-northstar/src/components/Tree/TreeItem.tsx Co-Authored-By: Marija Najdova <mnajdova@gmail.com> * Update packages/fluentui/react-northstar/src/components/Tree/TreeTitle.tsx Co-Authored-By: Marija Najdova <mnajdova@gmail.com> * Update packages/fluentui/react-northstar/src/themes/teams/components/Tree/treeTitleVariables.ts Co-Authored-By: Marija Najdova <mnajdova@gmail.com> * Update packages/fluentui/accessibility/src/behaviors/Tree/treeTitleBehavior.ts Co-Authored-By: Oleksandr Fediashov <alexander.mcgarret@gmail.com> * Update packages/fluentui/react-northstar/src/components/Tree/Tree.tsx Co-Authored-By: Oleksandr Fediashov <alexander.mcgarret@gmail.com> * Update packages/fluentui/react-northstar/src/components/Tree/TreeItem.tsx Co-Authored-By: Oleksandr Fediashov <alexander.mcgarret@gmail.com> * feat(Tree): Temp removing option to pass a custom checkbox to the selectable tree * Update packages/fluentui/docs/src/examples/components/Tree/Usage/TreeMultiselectExample.shorthand.tsx Co-Authored-By: Oleksandr Fediashov <alexander.mcgarret@gmail.com> * feat(Tree): Reduce over forEach * feat(Tree): Typo * Update packages/fluentui/accessibility/test/behaviors/treeTitleBehavior-test.tsx Co-Authored-By: Roman Sudarikov <pompomon@users.noreply.github.com> * feat(Tree): Enable select all nested children * feat(Tree): Fix select on right click * feat(Tree): Reverting non-related file * feat(Tree): Fix tree title behavior tests * feat(Tree): Fix behavior test for subTree * feat(Tree): Outline treetitle as block when selectable * feat(Tree): Custom Select Indicator * feat(Tree): Remove focus from custom checkbox * feat(Tree): fix select all * removing rtl types and fix if condition for not selectable item * adding example that will handle custom selectable parent and default * adding methods to check if all or some of children of a groups has been selected * handling incompleted group selected * adding icon for 3rd checkbox state * feat(Tree): Improve determinated state for selectable parent * feat(Tree): enable toggle parent select using keyboard * feat(Tree): indeterminated state url file * chore(treeTitleVariables): remove lodash * feat(Tree): border color * feat(Tree): add step * feat(Tree): Remove Dup File * feat(Tree): Right condition * feat(Tree): Adapt example * feat(Tree): Fix behavior test * feat(Tree): Toggle multiselection * feat(Tree): Toggle multiselection * feat(Tree): Remove keyboard import * feat(Tree): Fix conditions * feat(Tree): Fix test behaviors * feat(Tree): Remove select on enter * feat(Tree): Active items test * feat(Tree): Space select screenshot * feat(Tree): Review changes * feat(Tree): Fix test * feat(Tree): remove log * feat(Tree): steps * fix(Dropdown): don't clear value on same item selection (microsoft#12784) * fix same item selection * improve the unit test * changelog * improve code * hide/unhide legends (microsoft#12469) * adding new prop hideLegend in donut chart and line chart which decides to show or hide legends * Change files Co-authored-by: v-sivsar <v-sivsar@microsoft.com> * User/v sivsar/linchart line click (microsoft#12548) * adding onclick to the line in the line chart * Change files Co-authored-by: v-sivsar <v-sivsar@microsoft.com> * chore(TableRow): Convert to FC (microsoft#12797) * Convert TableRow to FC * Update Changlelog * Fix imports * Apply suggestions from code review Co-Authored-By: Marija Najdova <mnajdova@gmail.com> * Add TableRow onClick to handled props * Properly add onClick to handledProps Co-authored-by: Marija Najdova <mnajdova@gmail.com> * FocusZone: Adding preventDefaultWhenHandled to v7 version (microsoft#12636) #### Pull request checklist - [ ] Addresses an existing issue: Fixes #0000 - [x] Include a change request file using `$ yarn change` #### Description of changes This PR brings the `preventDefaultWhenHandled` prop from the v0 version of `FocusZone` to the v7 version. This prop, if true, prevents the default behavior of focus events. #### Focus areas to test (optional) * FocusZone: Updating comment for preventDefaultWhenHandled prop (microsoft#12830) #### Pull request checklist - [ ] Addresses an existing issue: Fixes #0000 - [x] Include a change request file using `$ yarn change` #### Description of changes `FocusZone`: Updating comment for `preventDefaultWhenHandled` prop to be consistent across both versions. #### Focus areas to test (optional) * feature: improve `compose()` signature (microsoft#12745) * feat: compose sig * wip * bump api * -cleanup * -changed compose tests * -fixed any * -fixed screener regressions * -fixed build issues -fixed screener * Update packages/react-compose/src/types.ts Co-Authored-By: Oleksandr Fediashov <olfedias@microsoft.com> * fix types in ButtonContent * -update api * -test updates * -ts fixes * -added shorthandConfig in Button component * -fixed compose test * -fix * Update packages/fluentui/react-northstar/src/components/Box/Box.tsx Co-Authored-By: Roman Sudarikov <pompomon@users.noreply.github.com> * Update packages/fluentui/react-northstar/src/components/Button/Button.tsx Co-Authored-By: Roman Sudarikov <pompomon@users.noreply.github.com> * fixes * wip * fix RefForward to handle `innerRef` changes * reverted Accordion changes * -tslint disable * -screener diff * -testing * -reverted changes * -better description * fix build * changelog entry * Update packages/fluentui/CHANGELOG.md Co-Authored-By: Oleksandr Fediashov <olfedias@microsoft.com> Co-authored-by: Marija Najdova <mnajdova@gmail.com> Co-authored-by: Roman Sudarikov <pompomon@users.noreply.github.com> * applying package updates * Callout: Updating positioning logic to account for dual screen scenarios (microsoft#12785) #### Pull request checklist - [ ] Addresses an existing issue: Fixes #0000 - [x] Include a change request file using `$ yarn change` #### Description of changes This PR updates the positioning logic in the `Callout` component to make it dual-screen aware using the new `windowSegments` API. Components using `Callout` underneath such as `ContextualMenu` are also updated. This PR also adds a bunch of unit tests checking that the correct bounds for the screen segments are being calculated. __Before:__  __After:__  #### Focus areas to test * feat(Tree): steps Co-authored-by: Michal <michal.frdlik@hotmail.com> Co-authored-by: Milan Turon <mituron@microsoft.com> Co-authored-by: Milan Turon <milanturon@centrum.cz> Co-authored-by: Charles Assuncao <chassunc@microsoft.com> Co-authored-by: Roman Sudarikov <pompomon@users.noreply.github.com> Co-authored-by: Marija Najdova <mnajdova@gmail.com> Co-authored-by: Oleksandr Fediashov <alexander.mcgarret@gmail.com> Co-authored-by: Charles Assuncao <charles.assuncao@microsoft.com> Co-authored-by: Silviu Alexandru Avram <silviu.avram91@gmail.com> Co-authored-by: shivasai09 <33802398+shivasai09@users.noreply.github.com> Co-authored-by: v-sivsar <v-sivsar@microsoft.com> Co-authored-by: Makoto Morimoto <humbertomakotomorimoto@gmail.com> Co-authored-by: Oleksandr Fediashov <olfedias@microsoft.com> Co-authored-by: Fluent UI Build <fluentui-internal@service.microsoft.com>
miroslavstastny
pushed a commit
that referenced
this pull request
May 13, 2020
* fix same item selection * improve the unit test * changelog * improve code (cherry picked from commit 3b8eec1)
8 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Pull request checklist
$ yarn changeDescription of changes
Checks on selection if item coming from Downshift is not undefined. This is the case when the same item is selected. If it's the same item, will not update
valueor trigger a selection a11y message.Focus areas to test
(optional)
Microsoft Reviewers: Open in CodeFlow