From f0f80d93fe50a46e584aa0aee9458219842eacaa Mon Sep 17 00:00:00 2001 From: Boston Cartwright Date: Mon, 30 Nov 2020 16:46:21 -0500 Subject: [PATCH 1/3] feat(filterable-multiselect): add onMenuChange event --- .all-contributorsrc | 9 ++ README.md | 127 +++++++++--------- .../__snapshots__/PublicAPI-test.js.snap | 3 + .../MultiSelect/FilterableMultiSelect.js | 48 ++++--- .../MultiSelect/MultiSelect-story.js | 3 + .../__tests__/FilterableMultiSelect-test.js | 1 + .../FilterableMultiSelect-test.js.snap | 1 + 7 files changed, 110 insertions(+), 82 deletions(-) diff --git a/.all-contributorsrc b/.all-contributorsrc index 12ccbf983ae6..6f68dd2a621c 100644 --- a/.all-contributorsrc +++ b/.all-contributorsrc @@ -628,6 +628,15 @@ "contributions": [ "code" ] + }, + { + "login": "munkurious", + "name": "Boston Cartwright", + "avatar_url": "https://avatars0.githubusercontent.com/u/2187109?v=4", + "profile": "https://github.com/munkurious", + "contributions": [ + "code" + ] } ], "commitConvention": "none" diff --git a/README.md b/README.md index 938617ffcf13..3e0b537ac908 100644 --- a/README.md +++ b/README.md @@ -83,88 +83,89 @@ check out our [Contributing Guide](/.github/CONTRIBUTING.md) and our - - - - - - - + + + + + + + - - - - - - - + + + + + + + - - - - - - - + + + + + + + - - - - - - - + + + + + + + - - - - - - - + + + + + + + - - - - - - - + + + + + + + - - - - - - - + + + + + + + - - - - - - - + + + + + + + - - - - - - + + + + + + +

TJ Egan
πŸ’» πŸ“– πŸš‡ πŸ‘€ ️️️️♿️
Josh Black
πŸ’» πŸ“– πŸš‡ πŸ‘€ ️️️️♿️
Alessandra Davila
πŸ’» πŸ“– πŸ‘€ ️️️️♿️
DAK
πŸ’» πŸ“– πŸ‘€ ️️️️♿️
Andrea N. Cardona
πŸ’» πŸ“– πŸ‘€ ️️️️♿️
emyarod
πŸ’» πŸ“– πŸ‘€ ️️️️♿️
Josefina Mancilla
πŸ’» πŸ“– πŸ‘€ ️️️️♿️
TJ Egan
πŸ’» πŸ“– πŸš‡ πŸ‘€ ️️️️♿️
Josh Black
πŸ’» πŸ“– πŸš‡ πŸ‘€ ️️️️♿️
Alessandra Davila
πŸ’» πŸ“– πŸ‘€ ️️️️♿️
DAK
πŸ’» πŸ“– πŸ‘€ ️️️️♿️
Andrea N. Cardona
πŸ’» πŸ“– πŸ‘€ ️️️️♿️
emyarod
πŸ’» πŸ“– πŸ‘€ ️️️️♿️
Josefina Mancilla
πŸ’» πŸ“– πŸ‘€ ️️️️♿️

Vince Picone
πŸ’» πŸ“– πŸ‘€ ️️️️♿️
Ricardo Henriquez
πŸ’» πŸ“– πŸ‘€ ️️️️♿️
Scott Strubberg
πŸ’» πŸ“– πŸ‘€ ️️️️♿️
Alison Joseph
πŸ’» πŸ“– πŸ‘€ ️️️️♿️
Anna Gonzales
🎨 πŸ‘€
jillianhowarth
πŸ–‹ 🎨 πŸ‘€
Lauren Rice
🎨 πŸ‘€
Vince Picone
πŸ’» πŸ“– πŸ‘€ ️️️️♿️
Ricardo Henriquez
πŸ’» πŸ“– πŸ‘€ ️️️️♿️
Scott Strubberg
πŸ’» πŸ“– πŸ‘€ ️️️️♿️
Alison Joseph
πŸ’» πŸ“– πŸ‘€ ️️️️♿️
Anna Gonzales
🎨 πŸ‘€
jillianhowarth
πŸ–‹ 🎨 πŸ‘€
Lauren Rice
🎨 πŸ‘€

johnbister
🎨 πŸ‘€
Dominik Brugger
πŸ’» 🚧
Jan Hassel
πŸ’»
Alexander Lyon
πŸ’»
Rosie Z
πŸ’»
Abdul Rehman
πŸ’»
Nick Gong
πŸ’»
johnbister
🎨 πŸ‘€
Dominik Brugger
πŸ’» 🚧
Jan Hassel
πŸ’»
Alexander Lyon
πŸ’»
Rosie Z
πŸ’»
Abdul Rehman
πŸ’»
Nick Gong
πŸ’»

Nishith P
πŸ“–
Eric Charpentier
πŸ’»
Luiza Mendes
πŸ’» 🚧
Akmal Hakimi Mohd Zamri
πŸ’»
sanjitbauli
πŸ“–
Laszlo Moczo
πŸ’»
LMapes
πŸ–‹ πŸ“–
Nishith P
πŸ“–
Eric Charpentier
πŸ’»
Luiza Mendes
πŸ’» 🚧
Akmal Hakimi Mohd Zamri
πŸ’»
sanjitbauli
πŸ“–
Laszlo Moczo
πŸ’»
LMapes
πŸ–‹ πŸ“–

conradennis
🎨
Eric Liu
πŸ’» πŸ“–
Richard VΕ‘ianskΓ½
πŸ’»
Lee Chase
πŸ’» πŸ“–
Anton
πŸ’»
Panpan Lin
πŸ“–
Ashley Harrison
πŸ’»
conradennis
🎨
Eric Liu
πŸ’» πŸ“–
Richard VΕ‘ianskΓ½
πŸ’»
Lee Chase
πŸ’» πŸ“–
Anton
πŸ’»
Panpan Lin
πŸ“–
Ashley Harrison
πŸ’»

Jen Downs
πŸ’» πŸ“– ️️️️♿️
Taylor Jones
πŸ’» πŸ“– ️️️️♿️
MIchael Dudley
🎨
David Bradshaw
πŸ’»
Simon Finney
πŸ’» ️️️️♿️
Attila Bartha
πŸ’»
λ°°ν•˜λžŒ
πŸ’»
Jen Downs
πŸ’» πŸ“– ️️️️♿️
Taylor Jones
πŸ’» πŸ“– ️️️️♿️
MIchael Dudley
🎨
David Bradshaw
πŸ’»
Simon Finney
πŸ’» ️️️️♿️
Attila Bartha
πŸ’»
λ°°ν•˜λžŒ
πŸ’»

Yohanna Gadelrab
πŸ“–
Akira Sudoh
πŸ’» πŸ“– ️️️️♿️ πŸš‡
Oyinkan Oyetunmibi
πŸ“–
pbenson322
πŸ“–
Abbey Hart
πŸ’» πŸ“– ️️️️♿️
Lucas
πŸ’»
Dylan Klohr
πŸ“–
Yohanna Gadelrab
πŸ“–
Akira Sudoh
πŸ’» πŸ“– ️️️️♿️ πŸš‡
Oyinkan Oyetunmibi
πŸ“–
pbenson322
πŸ“–
Abbey Hart
πŸ’» πŸ“– ️️️️♿️
Lucas
πŸ’»
Dylan Klohr
πŸ“–

Gilli Sigurdsson
🎨
kennylam
πŸ’» ️️️️♿️
SΓ©bastien
πŸ’»
Dusan Milko
πŸ’»
Taras Polovyi
πŸ’»
Chris Connors
🎨 πŸ“–
David Conner
πŸ’» ️️️️♿️
Gilli Sigurdsson
🎨
kennylam
πŸ’» ️️️️♿️
SΓ©bastien
πŸ’»
Dusan Milko
πŸ’»
Taras Polovyi
πŸ’»
Chris Connors
🎨 πŸ“–
David Conner
πŸ’» ️️️️♿️

Harish Mohanani
πŸ’»
Frivalszky-Mayer PΓ©ter
πŸ’» ️️️️♿️
s100
πŸ’»
Taranveer Virk
πŸ’»
Niall Cargill
πŸ“–
Matt Chapman
πŸ’»
Harish Mohanani
πŸ’»
Frivalszky-Mayer PΓ©ter
πŸ’» ️️️️♿️
s100
πŸ’»
Taranveer Virk
πŸ’»
Niall Cargill
πŸ“–
Matt Chapman
πŸ’»
Boston Cartwright
πŸ’»
- + diff --git a/packages/react/__tests__/__snapshots__/PublicAPI-test.js.snap b/packages/react/__tests__/__snapshots__/PublicAPI-test.js.snap index 03fa4526106e..90b7b7eddb8c 100644 --- a/packages/react/__tests__/__snapshots__/PublicAPI-test.js.snap +++ b/packages/react/__tests__/__snapshots__/PublicAPI-test.js.snap @@ -3564,6 +3564,9 @@ Map { "onChange": Object { "type": "func", }, + "onMenuChange": Object { + "type": "func", + }, "open": Object { "type": "bool", }, diff --git a/packages/react/src/components/MultiSelect/FilterableMultiSelect.js b/packages/react/src/components/MultiSelect/FilterableMultiSelect.js index a8f135117231..9ea22ca6f5ea 100644 --- a/packages/react/src/components/MultiSelect/FilterableMultiSelect.js +++ b/packages/react/src/components/MultiSelect/FilterableMultiSelect.js @@ -94,6 +94,12 @@ export default class FilterableMultiSelect extends React.Component { */ onChange: PropTypes.func, + /** + * `onMenuChange` is a utility for this controlled component to communicate to a + * consuming component that the menu was opened(`true`)/closed(`false`). + */ + onMenuChange: PropTypes.func, + /** * Initialize the component with an open(`true`)/closed(`false`) menu. */ @@ -184,17 +190,24 @@ export default class FilterableMultiSelect extends React.Component { } }; - handleOnToggleMenu = () => { + handleOnMenuChange = (isOpen) => { this.setState((state) => ({ - isOpen: !state.isOpen, + isOpen: isOpen ?? !state.isOpen, })); + if (this.props.onMenuChange) { + this.props.onMenuChange(isOpen); + } + }; + + handleOnToggleMenu = () => { + this.handleOnMenuChange(); }; handleOnOuterClick = () => { this.setState({ - isOpen: false, inputValue: '', }); + this.handleOnMenuChange(false); }; handleOnStateChange = (changes, downshift) => { @@ -211,32 +224,29 @@ export default class FilterableMultiSelect extends React.Component { case Downshift.stateChangeTypes.keyDownArrowDown: this.setState({ highlightedIndex: changes.highlightedIndex, - isOpen: true, }); + this.handleOnMenuChange(true); break; case Downshift.stateChangeTypes.keyDownEscape: case Downshift.stateChangeTypes.mouseUp: - this.setState({ isOpen: false }); + this.handleOnMenuChange(false); break; // Opt-in to some cases where we should be toggling the menu based on // a given key press or mouse handler // Reference: https://github.com/paypal/downshift/issues/206 case Downshift.stateChangeTypes.clickButton: - case Downshift.stateChangeTypes.keyDownSpaceButton: - this.setState(() => { - let nextIsOpen = changes.isOpen || false; - if (changes.isOpen === false) { - // If Downshift is trying to close the menu, but we know the input - // is the active element in thedocument, then keep the menu open - if (this.inputNode === document.activeElement) { - nextIsOpen = true; - } + case Downshift.stateChangeTypes.keyDownSpaceButton: { + let nextIsOpen = changes.isOpen || false; + if (changes.isOpen === false) { + // If Downshift is trying to close the menu, but we know the input + // is the active element in the document, then keep the menu open + if (this.inputNode === document.activeElement) { + nextIsOpen = true; } - return { - isOpen: nextIsOpen, - }; - }); + } + this.handleOnMenuChange(nextIsOpen); break; + } } }; @@ -254,9 +264,9 @@ export default class FilterableMultiSelect extends React.Component { } return { inputValue: inputValue || '', - isOpen: Boolean(inputValue) || this.state.isOpen, }; }); + this.handleOnMenuChange(Boolean(inputValue) || this.state.isOpen); }; clearInputValue = (event) => { diff --git a/packages/react/src/components/MultiSelect/MultiSelect-story.js b/packages/react/src/components/MultiSelect/MultiSelect-story.js index a2260db4e851..555136f59e4c 100644 --- a/packages/react/src/components/MultiSelect/MultiSelect-story.js +++ b/packages/react/src/components/MultiSelect/MultiSelect-story.js @@ -197,6 +197,9 @@ export const _Filterable = withReadme(readme, () => { placeholder={defaultPlaceholder} translateWithId={(id) => listBoxMenuIconTranslationIds[id]} selectionFeedback={selectionFeedback} + onMenuChange={(e) => { + multiSelectProps.onMenuChange(e); + }} /> ); diff --git a/packages/react/src/components/MultiSelect/__tests__/FilterableMultiSelect-test.js b/packages/react/src/components/MultiSelect/__tests__/FilterableMultiSelect-test.js index ecacc2b76962..94e3a0ac1b27 100644 --- a/packages/react/src/components/MultiSelect/__tests__/FilterableMultiSelect-test.js +++ b/packages/react/src/components/MultiSelect/__tests__/FilterableMultiSelect-test.js @@ -29,6 +29,7 @@ describe('MultiSelect.Filterable', () => { items: generateItems(5, generateGenericItem), initialSelectedItems: [], onChange: jest.fn(), + onMenuChange: jest.fn(), placeholder: 'Placeholder...', }; }); diff --git a/packages/react/src/components/MultiSelect/__tests__/__snapshots__/FilterableMultiSelect-test.js.snap b/packages/react/src/components/MultiSelect/__tests__/__snapshots__/FilterableMultiSelect-test.js.snap index 15dd9af58bc1..bd8a56b80b56 100644 --- a/packages/react/src/components/MultiSelect/__tests__/__snapshots__/FilterableMultiSelect-test.js.snap +++ b/packages/react/src/components/MultiSelect/__tests__/__snapshots__/FilterableMultiSelect-test.js.snap @@ -41,6 +41,7 @@ exports[`MultiSelect.Filterable should render 1`] = ` light={false} locale="en" onChange={[MockFunction]} + onMenuChange={[MockFunction]} open={false} placeholder="Placeholder..." selectionFeedback="top-after-reopen" From e8d02129305c39c897dfdc47e7d2975ffb601137 Mon Sep 17 00:00:00 2001 From: Boston Cartwright Date: Tue, 1 Dec 2020 10:52:51 -0500 Subject: [PATCH 2/3] feat(filterable-multiselect): fix onMenuChange event --- .../MultiSelect/FilterableMultiSelect.js | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/packages/react/src/components/MultiSelect/FilterableMultiSelect.js b/packages/react/src/components/MultiSelect/FilterableMultiSelect.js index 9ea22ca6f5ea..0b234c0a58e6 100644 --- a/packages/react/src/components/MultiSelect/FilterableMultiSelect.js +++ b/packages/react/src/components/MultiSelect/FilterableMultiSelect.js @@ -199,15 +199,11 @@ export default class FilterableMultiSelect extends React.Component { } }; - handleOnToggleMenu = () => { - this.handleOnMenuChange(); - }; - handleOnOuterClick = () => { this.setState({ inputValue: '', }); - this.handleOnMenuChange(false); + if (this.state.isOpen) this.handleOnMenuChange(false); }; handleOnStateChange = (changes, downshift) => { @@ -225,11 +221,11 @@ export default class FilterableMultiSelect extends React.Component { this.setState({ highlightedIndex: changes.highlightedIndex, }); - this.handleOnMenuChange(true); + if (!this.state.isOpen) this.handleOnMenuChange(true); break; case Downshift.stateChangeTypes.keyDownEscape: case Downshift.stateChangeTypes.mouseUp: - this.handleOnMenuChange(false); + if (this.state.isOpen) this.handleOnMenuChange(false); break; // Opt-in to some cases where we should be toggling the menu based on // a given key press or mouse handler @@ -244,7 +240,8 @@ export default class FilterableMultiSelect extends React.Component { nextIsOpen = true; } } - this.handleOnMenuChange(nextIsOpen); + if (this.state.isOpen !== nextIsOpen) + this.handleOnMenuChange(nextIsOpen); break; } } @@ -255,7 +252,7 @@ export default class FilterableMultiSelect extends React.Component { }; handleOnInputValueChange = (inputValue, { type }) => { - if (type === Downshift.stateChangeTypes.changeInput) + if (type === Downshift.stateChangeTypes.changeInput) { this.setState(() => { if (Array.isArray(inputValue)) { return { @@ -266,7 +263,9 @@ export default class FilterableMultiSelect extends React.Component { inputValue: inputValue || '', }; }); - this.handleOnMenuChange(Boolean(inputValue) || this.state.isOpen); + if (Boolean(inputValue) !== this.state.isOpen) + this.handleOnMenuChange(Boolean(inputValue)); + } }; clearInputValue = (event) => { From 671d5db1b36926acd7193f38e42521624f506bfe Mon Sep 17 00:00:00 2001 From: Boston Cartwright Date: Tue, 1 Dec 2020 11:14:58 -0500 Subject: [PATCH 3/3] Update packages/react/src/components/MultiSelect/FilterableMultiSelect.js Co-authored-by: emyarod --- .../react/src/components/MultiSelect/FilterableMultiSelect.js | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/react/src/components/MultiSelect/FilterableMultiSelect.js b/packages/react/src/components/MultiSelect/FilterableMultiSelect.js index 0b234c0a58e6..6c9968b47ad8 100644 --- a/packages/react/src/components/MultiSelect/FilterableMultiSelect.js +++ b/packages/react/src/components/MultiSelect/FilterableMultiSelect.js @@ -203,7 +203,6 @@ export default class FilterableMultiSelect extends React.Component { this.setState({ inputValue: '', }); - if (this.state.isOpen) this.handleOnMenuChange(false); }; handleOnStateChange = (changes, downshift) => {