From 1abc27b27fbe86d838dd83ec0a216557eca9d66d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafa=C5=82=20Cie=C5=9Blak?= Date: Tue, 25 Apr 2023 12:58:02 +0200 Subject: [PATCH 1/8] Remove closeAndResetInput --- .../teleterm/src/ui/Search/SearchBar.test.tsx | 1 - .../teleterm/src/ui/Search/SearchContext.tsx | 7 ------- .../teleterm/src/ui/Search/pickers/ActionPicker.tsx | 10 ++++------ .../src/ui/Search/pickers/ParameterPicker.tsx | 12 ++++++------ 4 files changed, 10 insertions(+), 20 deletions(-) diff --git a/web/packages/teleterm/src/ui/Search/SearchBar.test.tsx b/web/packages/teleterm/src/ui/Search/SearchBar.test.tsx index 02c7a8f5e69bd..1d48c7928ac04 100644 --- a/web/packages/teleterm/src/ui/Search/SearchBar.test.tsx +++ b/web/packages/teleterm/src/ui/Search/SearchBar.test.tsx @@ -323,7 +323,6 @@ const getMockedSearchContext = (): SearchContext.SearchContext => ({ isOpen: true, open: () => {}, close: () => {}, - closeAndResetInput: () => {}, resetInput: () => {}, changeActivePicker: () => {}, onInputValueChange: () => {}, diff --git a/web/packages/teleterm/src/ui/Search/SearchContext.tsx b/web/packages/teleterm/src/ui/Search/SearchContext.tsx index ad65425ddf31a..b9b2fb9747292 100644 --- a/web/packages/teleterm/src/ui/Search/SearchContext.tsx +++ b/web/packages/teleterm/src/ui/Search/SearchContext.tsx @@ -38,7 +38,6 @@ export interface SearchContext { isOpen: boolean; open(fromElement?: Element): void; close(): void; - closeAndResetInput(): void; resetInput(): void; setFilter(filter: SearchFilter): void; removeFilter(filter: SearchFilter): void; @@ -82,11 +81,6 @@ export const SearchContextProvider: FC = props => { } }, []); - const closeAndResetInput = useCallback(() => { - close(); - setInputValue(''); - }, [close]); - const resetInput = useCallback(() => { setInputValue(''); }, []); @@ -197,7 +191,6 @@ export const SearchContextProvider: FC = props => { isOpen, open, close, - closeAndResetInput, pauseUserInteraction, addWindowEventListener, }} diff --git a/web/packages/teleterm/src/ui/Search/pickers/ActionPicker.tsx b/web/packages/teleterm/src/ui/Search/pickers/ActionPicker.tsx index b399bff3d2088..4f1849edcfe4f 100644 --- a/web/packages/teleterm/src/ui/Search/pickers/ActionPicker.tsx +++ b/web/packages/teleterm/src/ui/Search/pickers/ActionPicker.tsx @@ -69,7 +69,6 @@ export function ActionPicker(props: { input: ReactElement }) { close, inputValue, resetInput, - closeAndResetInput, filters, removeFilter, addWindowEventListener, @@ -112,17 +111,16 @@ export function ActionPicker(props: { input: ReactElement }) { // Overall, the context should probably encapsulate more logic so that the components don't // have to worry about low-level stuff such as input state. Input state already lives in the // search context so it should be managed from there, if possible. - if (action.preventAutoClose === true) { - resetInput(); - } else { - closeAndResetInput(); + resetInput(); + if (!action.preventAutoClose) { + close(); } } if (action.type === 'parametrized-action') { changeActivePicker(getParameterPicker(action)); } }, - [changeActivePicker, closeAndResetInput, resetInput] + [changeActivePicker, close, resetInput] ); const filterButtons = filters.map(s => { diff --git a/web/packages/teleterm/src/ui/Search/pickers/ParameterPicker.tsx b/web/packages/teleterm/src/ui/Search/pickers/ParameterPicker.tsx index a33c437b3f009..b37154e1916fe 100644 --- a/web/packages/teleterm/src/ui/Search/pickers/ParameterPicker.tsx +++ b/web/packages/teleterm/src/ui/Search/pickers/ParameterPicker.tsx @@ -37,7 +37,7 @@ interface ParameterPickerProps { export function ParameterPicker(props: ParameterPickerProps) { const { inputValue, - closeAndResetInput, + close, changeActivePicker, resetInput, addWindowEventListener, @@ -62,13 +62,13 @@ export function ParameterPicker(props: ParameterPickerProps) { const onPick = useCallback( (item: string) => { props.action.perform(item); - if (props.action.preventAutoClose === true) { - resetInput(); - } else { - closeAndResetInput(); + + resetInput(); + if (!props.action.preventAutoClose) { + close(); } }, - [closeAndResetInput, resetInput, props.action] + [close, resetInput, props.action] ); const onBack = useCallback(() => { From bf97cdd1561e0a4c91b2ef6f4d2a2701c04627c4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafa=C5=82=20Cie=C5=9Blak?= Date: Tue, 25 Apr 2023 12:58:02 +0200 Subject: [PATCH 2/8] Remove unnecessary input focus from `open` As the comment in SearchBar explains, the focus wasn't actually achieving anything. I added it only because I was testing SearchContext in separation and depended on the input getting focused in the test. This doesn't reflect how the input is actually used so I adjusted the test. --- web/packages/teleterm/src/ui/Search/SearchBar.tsx | 6 +++++- web/packages/teleterm/src/ui/Search/SearchContext.test.tsx | 7 ++++--- web/packages/teleterm/src/ui/Search/SearchContext.tsx | 7 ++++--- 3 files changed, 13 insertions(+), 7 deletions(-) diff --git a/web/packages/teleterm/src/ui/Search/SearchBar.tsx b/web/packages/teleterm/src/ui/Search/SearchBar.tsx index dcd0c37c58be5..fe72479eea36d 100644 --- a/web/packages/teleterm/src/ui/Search/SearchBar.tsx +++ b/web/packages/teleterm/src/ui/Search/SearchBar.tsx @@ -128,7 +128,11 @@ function SearchBar() { )} {isOpen && ( } /> )} diff --git a/web/packages/teleterm/src/ui/Search/SearchContext.test.tsx b/web/packages/teleterm/src/ui/Search/SearchContext.test.tsx index 31cce8c34b2f5..6ebb95ac39316 100644 --- a/web/packages/teleterm/src/ui/Search/SearchContext.test.tsx +++ b/web/packages/teleterm/src/ui/Search/SearchContext.test.tsx @@ -146,11 +146,12 @@ describe('addWindowEventListener', () => { describe('open', () => { it('manages the focus properly when called with no arguments', () => { const SearchInput = () => { - const { inputRef, open, close } = useSearchContext(); + const { inputRef, isOpen, open, close } = useSearchContext(); return ( <> +
{String(isOpen)}