From 58cf605640796d012e6f4d203a4b8ec18f90da4f Mon Sep 17 00:00:00 2001 From: Said MARAR Date: Fri, 17 Feb 2023 00:11:47 +0100 Subject: [PATCH 1/4] [Autocomplete] fix scroll on mobile when options list is changed --- .../api-docs/autocomplete/autocomplete.json | 2 +- .../AutocompleteUnstyled/useAutocomplete.d.ts | 2 +- .../AutocompleteUnstyled/useAutocomplete.js | 13 +++++- .../mui-joy/src/Autocomplete/Autocomplete.tsx | 2 +- .../src/Autocomplete/Autocomplete.js | 2 +- .../src/Autocomplete/Autocomplete.test.js | 42 +++++++++++++++++++ 6 files changed, 57 insertions(+), 6 deletions(-) diff --git a/docs/translations/api-docs/autocomplete/autocomplete.json b/docs/translations/api-docs/autocomplete/autocomplete.json index 7a2fe237d9a903..d6081cacfc1074 100644 --- a/docs/translations/api-docs/autocomplete/autocomplete.json +++ b/docs/translations/api-docs/autocomplete/autocomplete.json @@ -43,7 +43,7 @@ "noOptionsText": "Text to display when there are no options.
For localization purposes, you can use the provided translations.", "onChange": "Callback fired when the value changes.

Signature:
function(event: React.SyntheticEvent, value: T | Array<T>, reason: string, details?: string) => void
event: The event source of the callback.
value: The new value of the component.
reason: One of "createOption", "selectOption", "removeOption", "blur" or "clear".", "onClose": "Callback fired when the popup requests to be closed. Use in controlled mode (see open).

Signature:
function(event: React.SyntheticEvent, reason: string) => void
event: The event source of the callback.
reason: Can be: "toggleInput", "escape", "selectOption", "removeOption", "blur".", - "onHighlightChange": "Callback fired when the highlight option changes.

Signature:
function(event: React.SyntheticEvent, option: T, reason: string) => void
event: The event source of the callback.
option: The highlighted option.
reason: Can be: "keyboard", "auto", "mouse".", + "onHighlightChange": "Callback fired when the highlight option changes.

Signature:
function(event: React.SyntheticEvent, option: T, reason: string) => void
event: The event source of the callback.
option: The highlighted option.
reason: Can be: "keyboard", "auto", "mouse", "touch".", "onInputChange": "Callback fired when the input value changes.

Signature:
function(event: React.SyntheticEvent, value: string, reason: string) => void
event: The event source of the callback.
value: The new value of the text input.
reason: Can be: "input" (user input), "reset" (programmatic change), "clear".", "onOpen": "Callback fired when the popup requests to be opened. Use in controlled mode (see open).

Signature:
function(event: React.SyntheticEvent) => void
event: The event source of the callback.", "open": "If true, the component is shown.", diff --git a/packages/mui-base/src/AutocompleteUnstyled/useAutocomplete.d.ts b/packages/mui-base/src/AutocompleteUnstyled/useAutocomplete.d.ts index 1f7a47eacbcffd..c653de8ae7e05a 100644 --- a/packages/mui-base/src/AutocompleteUnstyled/useAutocomplete.d.ts +++ b/packages/mui-base/src/AutocompleteUnstyled/useAutocomplete.d.ts @@ -233,7 +233,7 @@ export interface UseAutocompleteProps< * * @param {React.SyntheticEvent} event The event source of the callback. * @param {T} option The highlighted option. - * @param {string} reason Can be: `"keyboard"`, `"auto"`, `"mouse"`. + * @param {string} reason Can be: `"keyboard"`, `"auto"`, `"mouse"`, `"touch"`. */ onHighlightChange?: ( event: React.SyntheticEvent, diff --git a/packages/mui-base/src/AutocompleteUnstyled/useAutocomplete.js b/packages/mui-base/src/AutocompleteUnstyled/useAutocomplete.js index 7c6f0dc82caf0b..9525ab1ca66a85 100644 --- a/packages/mui-base/src/AutocompleteUnstyled/useAutocomplete.js +++ b/packages/mui-base/src/AutocompleteUnstyled/useAutocomplete.js @@ -376,7 +376,11 @@ export default function useAutocomplete(props) { // // Consider this API instead once it has a better browser support: // .scrollIntoView({ scrollMode: 'if-needed', block: 'nearest' }); - if (listboxNode.scrollHeight > listboxNode.clientHeight && reason !== 'mouse') { + if ( + listboxNode.scrollHeight > listboxNode.clientHeight && + reason !== 'mouse' && + reason !== 'touch' + ) { const element = option; const scrollBottom = listboxNode.clientHeight + listboxNode.scrollTop; @@ -973,7 +977,12 @@ export default function useAutocomplete(props) { }); }; - const handleOptionTouchStart = () => { + const handleOptionTouchStart = (event) => { + setHighlightedIndex({ + event, + index: Number(event.currentTarget.getAttribute('data-option-index')), + reason: 'touch', + }); isTouch.current = true; }; diff --git a/packages/mui-joy/src/Autocomplete/Autocomplete.tsx b/packages/mui-joy/src/Autocomplete/Autocomplete.tsx index 7d7c0e9082bffa..034a0ce5db4c4b 100644 --- a/packages/mui-joy/src/Autocomplete/Autocomplete.tsx +++ b/packages/mui-joy/src/Autocomplete/Autocomplete.tsx @@ -930,7 +930,7 @@ Autocomplete.propTypes /* remove-proptypes */ = { * * @param {React.SyntheticEvent} event The event source of the callback. * @param {T} option The highlighted option. - * @param {string} reason Can be: `"keyboard"`, `"auto"`, `"mouse"`. + * @param {string} reason Can be: `"keyboard"`, `"auto"`, `"mouse"`, `"touch"`. */ onHighlightChange: PropTypes.func, /** diff --git a/packages/mui-material/src/Autocomplete/Autocomplete.js b/packages/mui-material/src/Autocomplete/Autocomplete.js index 19e2615989e707..77e7222386b99b 100644 --- a/packages/mui-material/src/Autocomplete/Autocomplete.js +++ b/packages/mui-material/src/Autocomplete/Autocomplete.js @@ -966,7 +966,7 @@ Autocomplete.propTypes /* remove-proptypes */ = { * * @param {React.SyntheticEvent} event The event source of the callback. * @param {T} option The highlighted option. - * @param {string} reason Can be: `"keyboard"`, `"auto"`, `"mouse"`. + * @param {string} reason Can be: `"keyboard"`, `"auto"`, `"mouse"`, `"touch"`. */ onHighlightChange: PropTypes.func, /** diff --git a/packages/mui-material/src/Autocomplete/Autocomplete.test.js b/packages/mui-material/src/Autocomplete/Autocomplete.test.js index fd569a30b30eea..beeecae523ee55 100644 --- a/packages/mui-material/src/Autocomplete/Autocomplete.test.js +++ b/packages/mui-material/src/Autocomplete/Autocomplete.test.js @@ -249,6 +249,48 @@ describe('', () => { expect(getByRole('listbox')).to.have.property('scrollTop', 0); }); + // https://github.com/mui/material-ui/issues/36212 + it('should preserve scrollTop position of the listbox when adding new options on mobile', function test() { + if (/jsdom/.test(window.navigator.userAgent)) { + this.skip(); + } + function getOptions(count) { + return Array(count) + .fill('item') + .map((value, i) => value + i); + } + + const { getByRole, getAllByRole, setProps } = render( + } + ListboxProps={{ style: { padding: 0, maxHeight: '100px' } }} + PopperComponent={(props) => { + const { disablePortal, anchorEl, open, ...other } = props; + return ; + }} + />, + ); + const textbox = getByRole('combobox'); + const listbox = getByRole('listbox'); + + act(() => { + textbox.focus(); + }); + + expect(listbox).to.have.property('scrollTop', 0); + + const options = getAllByRole('option'); + act(() => { + fireEvent.touchStart(options[1]); + listbox.scrollBy(0, 60); + setProps({ options: getOptions(15) }); + }); + + expect(listbox).to.have.property('scrollTop', 60); + }); + it('should keep the current highlight if possible', () => { const { getByRole } = render( Date: Wed, 22 Feb 2023 23:59:31 +0100 Subject: [PATCH 2/4] update dcos api --- docs/translations/api-docs-joy/autocomplete/autocomplete.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/translations/api-docs-joy/autocomplete/autocomplete.json b/docs/translations/api-docs-joy/autocomplete/autocomplete.json index d9f978b8fd2435..0ed763c78265bd 100644 --- a/docs/translations/api-docs-joy/autocomplete/autocomplete.json +++ b/docs/translations/api-docs-joy/autocomplete/autocomplete.json @@ -32,7 +32,7 @@ "noOptionsText": "Text to display when there are no options.
For localization purposes, you can use the provided translations.", "onChange": "Callback fired when the value changes.

Signature:
function(event: React.SyntheticEvent, value: T | Array<T>, reason: string, details?: string) => void
event: The event source of the callback.
value: The new value of the component.
reason: One of "createOption", "selectOption", "removeOption", "blur" or "clear".", "onClose": "Callback fired when the popup requests to be closed. Use in controlled mode (see open).

Signature:
function(event: React.SyntheticEvent, reason: string) => void
event: The event source of the callback.
reason: Can be: "toggleInput", "escape", "selectOption", "removeOption", "blur".", - "onHighlightChange": "Callback fired when the highlight option changes.

Signature:
function(event: React.SyntheticEvent, option: T, reason: string) => void
event: The event source of the callback.
option: The highlighted option.
reason: Can be: "keyboard", "auto", "mouse".", + "onHighlightChange": "Callback fired when the highlight option changes.

Signature:
function(event: React.SyntheticEvent, option: T, reason: string) => void
event: The event source of the callback.
option: The highlighted option.
reason: Can be: "keyboard", "auto", "mouse", "touch".", "onInputChange": "Callback fired when the input value changes.

Signature:
function(event: React.SyntheticEvent, value: string, reason: string) => void
event: The event source of the callback.
value: The new value of the text input.
reason: Can be: "input" (user input), "reset" (programmatic change), "clear".", "onOpen": "Callback fired when the popup requests to be opened. Use in controlled mode (see open).

Signature:
function(event: React.SyntheticEvent) => void
event: The event source of the callback.", "open": "If true, the component is shown.", From 983bb0b49e8ec8da027925ba8dc8633da42b5dda Mon Sep 17 00:00:00 2001 From: Said MARAR Date: Thu, 2 Mar 2023 14:14:28 +0100 Subject: [PATCH 3/4] update code --- .../src/useAutocomplete/useAutocomplete.d.ts | 2 +- .../src/useAutocomplete/useAutocomplete.js | 6 +- .../src/Autocomplete/Autocomplete.test.js | 75 ++++++++----------- 3 files changed, 35 insertions(+), 48 deletions(-) diff --git a/packages/mui-base/src/useAutocomplete/useAutocomplete.d.ts b/packages/mui-base/src/useAutocomplete/useAutocomplete.d.ts index c653de8ae7e05a..8ee3b229f6b3be 100644 --- a/packages/mui-base/src/useAutocomplete/useAutocomplete.d.ts +++ b/packages/mui-base/src/useAutocomplete/useAutocomplete.d.ts @@ -299,7 +299,7 @@ export interface UseAutocompleteParameters< FreeSolo extends boolean | undefined, > extends UseAutocompleteProps {} -export type AutocompleteHighlightChangeReason = 'keyboard' | 'mouse' | 'auto'; +export type AutocompleteHighlightChangeReason = 'keyboard' | 'mouse' | 'auto' | 'touch'; export type AutocompleteChangeReason = | 'createOption' diff --git a/packages/mui-base/src/useAutocomplete/useAutocomplete.js b/packages/mui-base/src/useAutocomplete/useAutocomplete.js index 9525ab1ca66a85..e7b92c09a19661 100644 --- a/packages/mui-base/src/useAutocomplete/useAutocomplete.js +++ b/packages/mui-base/src/useAutocomplete/useAutocomplete.js @@ -376,11 +376,7 @@ export default function useAutocomplete(props) { // // Consider this API instead once it has a better browser support: // .scrollIntoView({ scrollMode: 'if-needed', block: 'nearest' }); - if ( - listboxNode.scrollHeight > listboxNode.clientHeight && - reason !== 'mouse' && - reason !== 'touch' - ) { + if (listboxNode.scrollHeight > listboxNode.clientHeight && reason !== 'mouse') { const element = option; const scrollBottom = listboxNode.clientHeight + listboxNode.scrollTop; diff --git a/packages/mui-material/src/Autocomplete/Autocomplete.test.js b/packages/mui-material/src/Autocomplete/Autocomplete.test.js index beeecae523ee55..97727633b631ed 100644 --- a/packages/mui-material/src/Autocomplete/Autocomplete.test.js +++ b/packages/mui-material/src/Autocomplete/Autocomplete.test.js @@ -249,48 +249,6 @@ describe('', () => { expect(getByRole('listbox')).to.have.property('scrollTop', 0); }); - // https://github.com/mui/material-ui/issues/36212 - it('should preserve scrollTop position of the listbox when adding new options on mobile', function test() { - if (/jsdom/.test(window.navigator.userAgent)) { - this.skip(); - } - function getOptions(count) { - return Array(count) - .fill('item') - .map((value, i) => value + i); - } - - const { getByRole, getAllByRole, setProps } = render( - } - ListboxProps={{ style: { padding: 0, maxHeight: '100px' } }} - PopperComponent={(props) => { - const { disablePortal, anchorEl, open, ...other } = props; - return ; - }} - />, - ); - const textbox = getByRole('combobox'); - const listbox = getByRole('listbox'); - - act(() => { - textbox.focus(); - }); - - expect(listbox).to.have.property('scrollTop', 0); - - const options = getAllByRole('option'); - act(() => { - fireEvent.touchStart(options[1]); - listbox.scrollBy(0, 60); - setProps({ options: getOptions(15) }); - }); - - expect(listbox).to.have.property('scrollTop', 60); - }); - it('should keep the current highlight if possible', () => { const { getByRole } = render( ', () => { expect(container.querySelectorAll(`.${chipClasses.root}`)).to.have.length(0); }); }); + + // https://github.com/mui/material-ui/issues/36212 + it('should preserve scrollTop position of the listbox when adding new options on mobile', function test() { + if (/jsdom/.test(window.navigator.userAgent)) { + this.skip(); + } + function getOptions(count) { + return Array(count) + .fill('item') + .map((value, i) => value + i); + } + + const { getByRole, getAllByRole, setProps } = render( + } + ListboxProps={{ style: { maxHeight: '100px' } }} + />, + ); + const listbox = getByRole('listbox'); + + expect(listbox).to.have.property('scrollTop', 0); + + const options = getAllByRole('option'); + act(() => { + fireEvent.touchStart(options[1]); + listbox.scrollBy(0, 60); + setProps({ options: getOptions(15) }); + }); + + expect(listbox).to.have.property('scrollTop', 60); + }); }); From 93ab6c50b99a6d556d6a728a457b66ab470bb319 Mon Sep 17 00:00:00 2001 From: ZeeshanTamboli Date: Mon, 6 Mar 2023 15:51:31 +0530 Subject: [PATCH 4/4] use only 10 options in test to keep it compact --- packages/mui-material/src/Autocomplete/Autocomplete.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/mui-material/src/Autocomplete/Autocomplete.test.js b/packages/mui-material/src/Autocomplete/Autocomplete.test.js index 2a041c3a1799d9..4fc2035b0ee9a2 100644 --- a/packages/mui-material/src/Autocomplete/Autocomplete.test.js +++ b/packages/mui-material/src/Autocomplete/Autocomplete.test.js @@ -2958,7 +2958,7 @@ describe('', () => { act(() => { fireEvent.touchStart(options[1]); listbox.scrollBy(0, 60); - setProps({ options: getOptions(15) }); + setProps({ options: getOptions(10) }); }); expect(listbox).to.have.property('scrollTop', 60);