From fdf0b21b7473e3416e7b5008afc8e400993289b0 Mon Sep 17 00:00:00 2001 From: sai6855 <60743144+sai6855@users.noreply.github.com> Date: Sun, 3 Sep 2023 20:50:14 +0530 Subject: [PATCH 01/16] fix --- packages/mui-base/src/Select/Select.test.tsx | 20 ++++++++++++++++++++ packages/mui-base/src/useList/listReducer.ts | 3 ++- 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/packages/mui-base/src/Select/Select.test.tsx b/packages/mui-base/src/Select/Select.test.tsx index 852a8cf090ca63..1c8bc6a6febce0 100644 --- a/packages/mui-base/src/Select/Select.test.tsx +++ b/packages/mui-base/src/Select/Select.test.tsx @@ -1234,4 +1234,24 @@ describe(' + + + + + , + ); + + const option = getAllByRole('option')[1]; + + // selects option + fireEvent.click(option); + expect(option).to.have.class(optionClasses.highlighted); + // deselects option + fireEvent.click(option); + expect(option).not.to.have.class(optionClasses.highlighted); + }); }); diff --git a/packages/mui-base/src/useList/listReducer.ts b/packages/mui-base/src/useList/listReducer.ts index bfce751beb5567..202b877bbaad62 100644 --- a/packages/mui-base/src/useList/listReducer.ts +++ b/packages/mui-base/src/useList/listReducer.ts @@ -217,10 +217,11 @@ function handleItemSelection>( // if the item is already selected, remove it from the selection, otherwise add it const newSelectedValues = toggleSelection(item, selectedValues, selectionMode, itemComparer); + const highlightedValue = selectedValues.includes(items[itemIndex]) ? null : item; return { ...state, selectedValues: newSelectedValues, - highlightedValue: item, + highlightedValue, }; } From 5dcc93557cff4ba368277f048a8c4fcd157da518 Mon Sep 17 00:00:00 2001 From: sai6855 <60743144+sai6855@users.noreply.github.com> Date: Sun, 3 Sep 2023 20:53:21 +0530 Subject: [PATCH 02/16] fix tests --- packages/mui-base/src/Select/Select.test.tsx | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/packages/mui-base/src/Select/Select.test.tsx b/packages/mui-base/src/Select/Select.test.tsx index 1c8bc6a6febce0..88292eed9d64ff 100644 --- a/packages/mui-base/src/Select/Select.test.tsx +++ b/packages/mui-base/src/Select/Select.test.tsx @@ -1245,13 +1245,16 @@ describe(', ); - const option = getAllByRole('option')[1]; + const options = getAllByRole('option'); + const secondOption = options[1]; + // it doesn't have highlighted class as it is not selected + expect(secondOption).not.to.have.class(optionClasses.highlighted); // selects option - fireEvent.click(option); - expect(option).to.have.class(optionClasses.highlighted); + fireEvent.click(secondOption); + expect(secondOption).to.have.class(optionClasses.highlighted); // deselects option - fireEvent.click(option); - expect(option).not.to.have.class(optionClasses.highlighted); + fireEvent.click(secondOption); + expect(secondOption).not.to.have.class(optionClasses.highlighted); }); }); From 4a81c8c83bc1195a11f8560452b28137978237b5 Mon Sep 17 00:00:00 2001 From: sai6855 <60743144+sai6855@users.noreply.github.com> Date: Sun, 3 Sep 2023 20:53:36 +0530 Subject: [PATCH 03/16] fix tests --- packages/mui-base/src/Select/Select.test.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/mui-base/src/Select/Select.test.tsx b/packages/mui-base/src/Select/Select.test.tsx index 88292eed9d64ff..f6f01758627fe5 100644 --- a/packages/mui-base/src/Select/Select.test.tsx +++ b/packages/mui-base/src/Select/Select.test.tsx @@ -1247,6 +1247,7 @@ describe('', () => { expect(renderOption4Spy.callCount).to.equal(0); }); - it('option should have highlighted class only when it is selected', () => { - const { getAllByRole } = render( - + + , + ); + + const select = getByRole('combobox'); + + act(() => { + select.focus(); + }); + + fireEvent.keyDown(select, { key: 'ArrowDown' }); + + const firstOption = getByRole('option'); + + expect(firstOption).to.have.class(optionClasses.highlighted); + }); + + it('except for 1st option - other options should have highlighted class only when option is selected', () => { + const { getAllByRole, getByRole } = render( + ', () => { , ); + const select = getByRole('combobox'); + + act(() => { + select.focus(); + }); + + fireEvent.keyDown(select, { key: 'ArrowDown' }); + const options = getAllByRole('option'); + const firstOption = options[0]; const secondOption = options[1]; + expect(firstOption).to.have.class(optionClasses.highlighted); // it doesn't have highlighted class as it is not selected expect(secondOption).not.to.have.class(optionClasses.highlighted); From 3317021d7a4aaaa593409156013675b787e7b876 Mon Sep 17 00:00:00 2001 From: sai6855 <60743144+sai6855@users.noreply.github.com> Date: Sun, 3 Sep 2023 21:46:17 +0530 Subject: [PATCH 05/16] change test name --- packages/mui-base/src/Select/Select.test.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/mui-base/src/Select/Select.test.tsx b/packages/mui-base/src/Select/Select.test.tsx index 6beb96ce3aa4a7..1e23844b1af8e1 100644 --- a/packages/mui-base/src/Select/Select.test.tsx +++ b/packages/mui-base/src/Select/Select.test.tsx @@ -1235,7 +1235,7 @@ describe(' @@ -1255,7 +1255,7 @@ describe(' From a78e78e5b55ca93062d5fbed8586b4ccf41f81bb Mon Sep 17 00:00:00 2001 From: sai6855 <60743144+sai6855@users.noreply.github.com> Date: Sun, 3 Sep 2023 21:52:29 +0530 Subject: [PATCH 06/16] line break --- packages/mui-base/src/useList/listReducer.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/mui-base/src/useList/listReducer.ts b/packages/mui-base/src/useList/listReducer.ts index 202b877bbaad62..1e3bbce3568367 100644 --- a/packages/mui-base/src/useList/listReducer.ts +++ b/packages/mui-base/src/useList/listReducer.ts @@ -218,6 +218,7 @@ function handleItemSelection>( const newSelectedValues = toggleSelection(item, selectedValues, selectionMode, itemComparer); const highlightedValue = selectedValues.includes(items[itemIndex]) ? null : item; + return { ...state, selectedValues: newSelectedValues, From 7176cbdc3c95fa2debab32802ea157c2397e67cf Mon Sep 17 00:00:00 2001 From: sai6855 <60743144+sai6855@users.noreply.github.com> Date: Mon, 4 Sep 2023 14:49:48 +0530 Subject: [PATCH 07/16] add reason --- packages/mui-base/src/useList/listReducer.ts | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/packages/mui-base/src/useList/listReducer.ts b/packages/mui-base/src/useList/listReducer.ts index 1e3bbce3568367..e30a3ceec10c67 100644 --- a/packages/mui-base/src/useList/listReducer.ts +++ b/packages/mui-base/src/useList/listReducer.ts @@ -204,6 +204,7 @@ function handleItemSelection>( item: ItemValue, state: State, context: ListActionContext, + reason: 'mouse' | 'keyboard', ): State { const { itemComparer, isItemDisabled, selectionMode, items } = context; const { selectedValues } = state; @@ -217,7 +218,8 @@ function handleItemSelection>( // if the item is already selected, remove it from the selection, otherwise add it const newSelectedValues = toggleSelection(item, selectedValues, selectionMode, itemComparer); - const highlightedValue = selectedValues.includes(items[itemIndex]) ? null : item; + const highlightedValue = + reason === 'mouse' && selectedValues.includes(items[itemIndex]) ? null : item; return { ...state, @@ -311,7 +313,7 @@ function handleKeyDown>( return state; } - return handleItemSelection(state.highlightedValue, state, context); + return handleItemSelection(state.highlightedValue, state, context, 'keyboard'); default: break; @@ -435,7 +437,7 @@ export function listReducer>( case ListActionTypes.keyDown: return handleKeyDown(action.key, state, context); case ListActionTypes.itemClick: - return handleItemSelection(action.item, state, context); + return handleItemSelection(action.item, state, context, 'mouse'); case ListActionTypes.blur: return handleBlur(state, context); case ListActionTypes.textNavigation: From 12d2c107b9967b444d90b29cc0dc37ef293ec7e6 Mon Sep 17 00:00:00 2001 From: sai6855 <60743144+sai6855@users.noreply.github.com> Date: Mon, 4 Sep 2023 15:31:53 +0530 Subject: [PATCH 08/16] highlight on hover --- packages/mui-base/src/useList/listReducer.ts | 16 ++++++++++++++++ packages/mui-base/src/useOption/useOption.ts | 1 + 2 files changed, 17 insertions(+) diff --git a/packages/mui-base/src/useList/listReducer.ts b/packages/mui-base/src/useList/listReducer.ts index e30a3ceec10c67..0d6492d7ddfaa2 100644 --- a/packages/mui-base/src/useList/listReducer.ts +++ b/packages/mui-base/src/useList/listReducer.ts @@ -427,6 +427,20 @@ function handleResetHighlight>( }; } +function handleItemHover>( + item: ItemValue, + state: State, +): State { + if (state.highlightedValue && state.selectedValues.includes(state.highlightedValue)) { + return state; + } + + return { + ...state, + highlightedValue: item, + }; +} + export function listReducer>( state: State, action: ListReducerAction & { context: ListActionContext }, @@ -438,6 +452,8 @@ export function listReducer>( return handleKeyDown(action.key, state, context); case ListActionTypes.itemClick: return handleItemSelection(action.item, state, context, 'mouse'); + case ListActionTypes.itemHover: + return handleItemHover(action.item, state); case ListActionTypes.blur: return handleBlur(state, context); case ListActionTypes.textNavigation: diff --git a/packages/mui-base/src/useOption/useOption.ts b/packages/mui-base/src/useOption/useOption.ts index 556e82506e3344..953629a5f4f3a6 100644 --- a/packages/mui-base/src/useOption/useOption.ts +++ b/packages/mui-base/src/useOption/useOption.ts @@ -26,6 +26,7 @@ export function useOption(params: UseOptionParameters): UseOptionR selected, } = useListItem({ item: value, + handlePointerOverEvents: true, }); const id = useId(idParam); From fbc9f41ac89a390afbfc9ac8efca46feceb79235 Mon Sep 17 00:00:00 2001 From: sai6855 <60743144+sai6855@users.noreply.github.com> Date: Mon, 4 Sep 2023 15:42:26 +0530 Subject: [PATCH 09/16] logic fix --- packages/mui-base/src/useList/listReducer.ts | 4 ---- 1 file changed, 4 deletions(-) diff --git a/packages/mui-base/src/useList/listReducer.ts b/packages/mui-base/src/useList/listReducer.ts index 0d6492d7ddfaa2..1bf5df5e6bfc25 100644 --- a/packages/mui-base/src/useList/listReducer.ts +++ b/packages/mui-base/src/useList/listReducer.ts @@ -431,10 +431,6 @@ function handleItemHover>( item: ItemValue, state: State, ): State { - if (state.highlightedValue && state.selectedValues.includes(state.highlightedValue)) { - return state; - } - return { ...state, highlightedValue: item, From 4fb78f76bb0fee606f754ac1cad95c6c3a08f6f0 Mon Sep 17 00:00:00 2001 From: sai6855 <60743144+sai6855@users.noreply.github.com> Date: Mon, 4 Sep 2023 19:52:14 +0530 Subject: [PATCH 10/16] add test --- packages/mui-base/src/Select/Select.test.tsx | 38 +++++++++++++++++++- 1 file changed, 37 insertions(+), 1 deletion(-) diff --git a/packages/mui-base/src/Select/Select.test.tsx b/packages/mui-base/src/Select/Select.test.tsx index 1e23844b1af8e1..20e7961a08a24c 100644 --- a/packages/mui-base/src/Select/Select.test.tsx +++ b/packages/mui-base/src/Select/Select.test.tsx @@ -1255,7 +1255,7 @@ describe(' @@ -1288,4 +1288,40 @@ describe(' + + + + + , + ); + + const select = getByRole('combobox'); + + act(() => { + select.focus(); + }); + + fireEvent.keyDown(select, { key: 'ArrowDown' }); + + const options = getAllByRole('option'); + const firstOption = options[0]; + const secondOption = options[1]; + + expect(firstOption).to.have.class(optionClasses.highlighted); + // it doesn't have highlighted class as it is not navigated yet + expect(secondOption).not.to.have.class(optionClasses.highlighted); + + fireEvent.keyDown(select, { key: 'ArrowDown' }); + expect(secondOption).to.have.class(optionClasses.highlighted); + + fireEvent.keyDown(select, { key: ' ' }); + expect(secondOption).to.have.class(optionClasses.highlighted); + + fireEvent.keyDown(select, { key: ' ' }); + expect(secondOption).to.have.class(optionClasses.highlighted); + }); }); From 4b77a93b9a291d7653793468241c737ae65fce4d Mon Sep 17 00:00:00 2001 From: sai6855 <60743144+sai6855@users.noreply.github.com> Date: Mon, 4 Sep 2023 20:08:05 +0530 Subject: [PATCH 11/16] add e2e test --- test/e2e/fixtures/Select/BaseSelect.tsx | 15 +++++++++++++++ test/e2e/index.test.ts | 24 ++++++++++++++++++++++++ 2 files changed, 39 insertions(+) create mode 100644 test/e2e/fixtures/Select/BaseSelect.tsx diff --git a/test/e2e/fixtures/Select/BaseSelect.tsx b/test/e2e/fixtures/Select/BaseSelect.tsx new file mode 100644 index 00000000000000..49baaed199d0cc --- /dev/null +++ b/test/e2e/fixtures/Select/BaseSelect.tsx @@ -0,0 +1,15 @@ +import * as React from 'react'; +import { Option, Select } from '@mui/base'; + +function BaseSelect() { + return ( + + ); +} + +export default BaseSelect; diff --git a/test/e2e/index.test.ts b/test/e2e/index.test.ts index 817a7ec0ac4813..e644a622865542 100644 --- a/test/e2e/index.test.ts +++ b/test/e2e/index.test.ts @@ -239,6 +239,30 @@ describe('e2e', () => { }); }); + describe('', () => { expect(secondOption).not.to.have.class(optionClasses.highlighted); }); - it('options should have highlighted class on keyboard navigation irrespective of option selection', () => { + it('deselected option should have highlighted class when deselected by keyboard', () => { const { getAllByRole, getByRole } = render( ', () => { // it doesn't have highlighted class as it is not navigated yet expect(secondOption).not.to.have.class(optionClasses.highlighted); - fireEvent.keyDown(select, { key: 'ArrowDown' }); + fireEvent.keyDown(select, { key: 'ArrowDown' }); // navigates to second option expect(secondOption).to.have.class(optionClasses.highlighted); - fireEvent.keyDown(select, { key: ' ' }); + fireEvent.keyDown(select, { key: ' ' }); // selects second option expect(secondOption).to.have.class(optionClasses.highlighted); - fireEvent.keyDown(select, { key: ' ' }); + fireEvent.keyDown(select, { key: ' ' }); // deselects second option expect(secondOption).to.have.class(optionClasses.highlighted); }); }); From 2625d2d548f21ca0960602f2ae7b1c9c4c68e215 Mon Sep 17 00:00:00 2001 From: sai6855 <60743144+sai6855@users.noreply.github.com> Date: Mon, 4 Sep 2023 21:02:54 +0530 Subject: [PATCH 14/16] add e2e test --- test/e2e/index.test.ts | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/test/e2e/index.test.ts b/test/e2e/index.test.ts index b1d0183217675a..0ef57966eed21b 100644 --- a/test/e2e/index.test.ts +++ b/test/e2e/index.test.ts @@ -240,7 +240,7 @@ describe('e2e', () => { }); describe('', () => { expect(secondOption).not.to.have.class(optionClasses.highlighted); }); - it('deselected option should have highlighted class when deselected by keyboard', () => { + it('deselected option should have highlighted class when option is deselected by keyboard', () => { const { getAllByRole, getByRole } = render(