diff --git a/.changeset/stale-bats-wink.md b/.changeset/stale-bats-wink.md new file mode 100644 index 00000000..4e18b185 --- /dev/null +++ b/.changeset/stale-bats-wink.md @@ -0,0 +1,10 @@ +--- +"@sap/guided-answers-extension-core": patch +"sap-guided-answers-extension": patch +"@sap/guided-answers-extension-webapp": patch +--- + +fix(ga): fix for #606 issue +- [x] add fix for pageSize issue +- [x] add cancel previous call to API before doing a new one +- [x] add debounce on search input onChange callback diff --git a/packages/core/src/guided-answers-api.ts b/packages/core/src/guided-answers-api.ts index 45b29a8f..3d8dabce 100644 --- a/packages/core/src/guided-answers-api.ts +++ b/packages/core/src/guided-answers-api.ts @@ -1,4 +1,4 @@ -import type { AxiosResponse } from 'axios'; +import type { AxiosResponse, CancelTokenSource } from 'axios'; import axios from 'axios'; import { default as xss } from 'xss'; import type { @@ -35,6 +35,7 @@ const FEEDBACK_COMMENT = `dtp/api/${VERSION}/feedback/comment`; const FEEDBACK_OUTCOME = `dtp/api/${VERSION}/feedback/outcome`; const DEFAULT_MAX_RESULTS = 9999; +const previousToken: CancelTokenSource[] = []; /** * Returns API to programmatically access Guided Answers. * @@ -53,7 +54,7 @@ export function getGuidedAnswerApi(options?: APIOptions): GuidedAnswerAPI { enhanceNode({ node: await getNodeById(apiHost, id), extensions, logger, ide }), getTreeById: async (id: GuidedAnswerTreeId): Promise => getTreeById(apiHost, id), getTrees: async (queryOptions?: GuidedAnswersQueryOptions): Promise => - getTrees(apiHost, queryOptions), + getTrees(apiHost, logger, queryOptions), getNodePath: async (nodeIdPath: GuidedAnswerNodeId[]): Promise => { let nodes = await getNodePath(apiHost, nodeIdPath); nodes = nodes.map((node) => enhanceNode({ node, extensions, logger, ide })); @@ -156,13 +157,19 @@ async function getTreeById(host: string, id: GuidedAnswerTreeId): Promise} A promise that resolves to the search result containing guided answer trees. + * @throws {Error} Throws an error if the query is a number or if the response does not contain a 'trees' array. */ -async function getTrees(host: string, queryOptions?: GuidedAnswersQueryOptions): Promise { +async function getTrees( + host: string, + logger: Logger, + queryOptions?: GuidedAnswersQueryOptions +): Promise { if (typeof queryOptions?.query === 'number') { throw Error( `Invalid search for tree with number. Please use string or function getTreeById() to get a tree by id` @@ -171,13 +178,40 @@ async function getTrees(host: string, queryOptions?: GuidedAnswersQueryOptions): const query = queryOptions?.query ? encodeURIComponent(`"${queryOptions.query}"`) : '*'; const urlGetParamString = convertQueryOptionsToGetParams(queryOptions?.filters, queryOptions?.paging); const url = `${host}${TREE_PATH}${query}${urlGetParamString}`; - const response: AxiosResponse = await axios.get(url); - const searchResult = response.data; - if (!Array.isArray(searchResult?.trees)) { - throw Error( - `Query result from call '${url}' does not contain property 'trees' as array. Received response: '${searchResult}'` - ); + + // Cancel the previous request if it exists + if (previousToken.length) { + const prev = previousToken.pop(); + prev?.cancel('Canceling previous request'); + } + + // Create a new CancelToken for the current request + const source = axios.CancelToken.source(); + previousToken.push(source); + + let searchResult: GuidedAnswerTreeSearchResult = { + resultSize: -1, + trees: [], + productFilters: [], + componentFilters: [] + }; + + try { + const response = await axios.get(url, { + cancelToken: source.token + }); + searchResult = response.data; + if (!Array.isArray(searchResult?.trees)) { + throw Error(`Query result from call '${url}' does not contain property 'trees' as array`); + } + } catch (error) { + if (axios.isCancel(error)) { + logger.logDebug(`Request canceled: '${error.message}'`); + } else { + throw error; + } } + return searchResult; } diff --git a/packages/core/test/guided-answers-api.test.ts b/packages/core/test/guided-answers-api.test.ts index f22d6693..3119284c 100644 --- a/packages/core/test/guided-answers-api.test.ts +++ b/packages/core/test/guided-answers-api.test.ts @@ -1,4 +1,5 @@ import axios from 'axios'; +import type { CancelTokenStatic } from 'axios'; import type { APIOptions, FeedbackCommentPayload, @@ -12,6 +13,7 @@ import { getGuidedAnswerApi } from '../src'; jest.mock('axios'); const mockedAxios = axios as jest.Mocked; +type Canceler = (message?: string) => void; const currentVersion = getGuidedAnswerApi().getApiInfo().version; describe('Guided Answers Api: getApiInfo()', () => { @@ -24,6 +26,31 @@ describe('Guided Answers Api: getApiInfo()', () => { }); describe('Guided Answers Api: getTrees()', () => { + /** + * Class representing a CancelToken. + */ + class CancelToken { + /** + * Creates a cancel token source. + * @returns {Object} An object containing the cancel function and token. + */ + public static source() { + const cancel: Canceler = jest.fn(); + const token = new CancelToken(); + return { + cancel, + token + }; + } + } + + mockedAxios.CancelToken = { + source: jest.fn(() => ({ + cancel: jest.fn(), + token: new CancelToken() + })) + } as any; + beforeEach(() => { jest.clearAllMocks(); }); @@ -67,6 +94,7 @@ describe('Guided Answers Api: getTrees()', () => { componentFilters: [{ COMPONENT: 'C1', COUNT: 1 }], productFilters: [{ PRODUCT: 'P_one', COUNT: 1 }] }; + let requestUrl = ''; mockedAxios.get.mockImplementation((url) => { requestUrl = url; diff --git a/packages/ide-extension/src/panel/guidedAnswersPanel.ts b/packages/ide-extension/src/panel/guidedAnswersPanel.ts index f22e19c4..c766e604 100644 --- a/packages/ide-extension/src/panel/guidedAnswersPanel.ts +++ b/packages/ide-extension/src/panel/guidedAnswersPanel.ts @@ -243,14 +243,16 @@ export class GuidedAnswersPanel { } this.loadingTimeout = setTimeout(() => { this.postActionToWebview(updateNetworkStatus('LOADING')); - }, 2000); + }, 50); } try { const trees = await this.guidedAnswerApi.getTrees(queryOptions); if (this.loadingTimeout) { clearTimeout(this.loadingTimeout); } - this.postActionToWebview(updateNetworkStatus('OK')); + if (trees.resultSize !== -1) { + this.postActionToWebview(updateNetworkStatus('OK')); + } return trees; } catch (e) { if (this.loadingTimeout) { diff --git a/packages/webapp/package.json b/packages/webapp/package.json index ba90168d..2ea8a962 100644 --- a/packages/webapp/package.json +++ b/packages/webapp/package.json @@ -22,10 +22,12 @@ "@testing-library/dom": "8.20.1", "@testing-library/jest-dom": "6.2.0", "@testing-library/react": "12.1.5", + "@types/lodash": "4.17.7", "@types/react": "17.0.62", "@types/react-copy-to-clipboard": "5.0.7", "@types/react-dom": "17.0.20", "@types/react-redux": "7.1.33", + "@types/redux-logger": "3.0.9", "@types/redux-mock-store": "1.0.6", "autoprefixer": "10.4.16", "esbuild": "0.19.11", @@ -37,6 +39,7 @@ "i18next": "23.7.16", "jest-css-modules-transform": "4.4.2", "jest-environment-jsdom": "29.7.0", + "lodash": "4.17.21", "path": "0.12.7", "postcss": "8.4.33", "react": "16.14.0", @@ -44,6 +47,7 @@ "react-i18next": "13.2.2", "react-redux": "8.1.2", "redux": "4.2.1", + "redux-logger": "3.0.6", "redux-mock-store": "1.5.4", "uuid": "9.0.1" }, diff --git a/packages/webapp/src/webview/state/middleware.ts b/packages/webapp/src/webview/state/middleware.ts index a7a292d6..1d71ec75 100644 --- a/packages/webapp/src/webview/state/middleware.ts +++ b/packages/webapp/src/webview/state/middleware.ts @@ -1,5 +1,6 @@ import i18next from 'i18next'; import type { Middleware, MiddlewareAPI, Dispatch, Action } from 'redux'; +import { createLogger } from 'redux-logger'; import type { GuidedAnswerActions } from '@sap/guided-answers-extension-types'; import { GO_TO_PREVIOUS_PAGE, @@ -40,7 +41,6 @@ export const communicationMiddleware: Middleware< // Add event handler, this will dispatch incoming state updates window.addEventListener('message', (event: MessageEvent) => { if (event.origin === window.origin) { - console.log(i18next.t('MESSAGE_RECEIVED'), event); if (event.data && typeof event.data.type === 'string') { store.dispatch(event.data); } @@ -58,7 +58,6 @@ export const communicationMiddleware: Middleware< action = next(action); if (action && typeof action.type === 'string') { window.vscode.postMessage(action); - console.log(i18next.t('REACT_ACTION_POSTED'), action); } return action; }; @@ -81,6 +80,7 @@ const allowedTelemetryActions = new Set([ UPDATE_BOOKMARKS, SYNCHRONIZE_BOOKMARK ]); + export const telemetryMiddleware: Middleware< Dispatch, AppState, @@ -115,3 +115,7 @@ export const restoreMiddleware: Middleware, AppSta return action; }; }; + +export const loggerMiddleware = createLogger({ + duration: true +}); diff --git a/packages/webapp/src/webview/state/store.ts b/packages/webapp/src/webview/state/store.ts index 38653454..32ce1bb3 100644 --- a/packages/webapp/src/webview/state/store.ts +++ b/packages/webapp/src/webview/state/store.ts @@ -1,6 +1,6 @@ import { configureStore } from '@reduxjs/toolkit'; import { bindActionCreators } from 'redux'; -import { telemetryMiddleware, communicationMiddleware, restoreMiddleware } from './middleware'; +import { telemetryMiddleware, communicationMiddleware, restoreMiddleware, loggerMiddleware } from './middleware'; import { getInitialState, reducer } from './reducers'; import * as AllActions from './actions'; @@ -8,7 +8,7 @@ export const store = configureStore({ reducer, preloadedState: getInitialState(), devTools: false, - middleware: [communicationMiddleware, telemetryMiddleware, restoreMiddleware] + middleware: [communicationMiddleware, telemetryMiddleware, restoreMiddleware, loggerMiddleware] }); // bind actions to store diff --git a/packages/webapp/src/webview/ui/components/App/App.tsx b/packages/webapp/src/webview/ui/components/App/App.tsx index e9ffc22e..99d07c85 100644 --- a/packages/webapp/src/webview/ui/components/App/App.tsx +++ b/packages/webapp/src/webview/ui/components/App/App.tsx @@ -26,28 +26,15 @@ initIcons(); export function App(): ReactElement { const appState = useSelector((state) => state); useEffect(() => { - const resultsContainer = document.getElementById('results-container'); - if (!resultsContainer) { - return undefined; - } - //tree-item element height is ~100px, using 50px here to be on the safe side. + //tree-item element height is ~100px, using 50px here to be on the safe side. Header uses ~300, minimum page size is 5. const setPageSizeByContainerHeight = (pxHeight: number): void => { - actions.setPageSize(Math.ceil(pxHeight / 50)); + actions.setPageSize(Math.max(5, Math.ceil((pxHeight - 300) / 50))); }; - const resizeObserver = new ResizeObserver((entries: ResizeObserverEntry[]) => { - const containerEntry = entries.find((entry) => entry?.target?.id === 'results-container'); - if (containerEntry) { - setPageSizeByContainerHeight(containerEntry.contentRect.height); - } - }); - // Set initial page size - setPageSizeByContainerHeight(resultsContainer.clientHeight); - resizeObserver.observe(resultsContainer); - return () => { - if (resizeObserver) { - resizeObserver.unobserve(resultsContainer); - } + window.onresize = () => { + setPageSizeByContainerHeight(window.innerHeight); }; + setPageSizeByContainerHeight(window.innerHeight); + return undefined; }, []); function fetchData() { diff --git a/packages/webapp/src/webview/ui/components/Header/SearchField/SearchField.tsx b/packages/webapp/src/webview/ui/components/Header/SearchField/SearchField.tsx index 6f76687c..cc9571fd 100644 --- a/packages/webapp/src/webview/ui/components/Header/SearchField/SearchField.tsx +++ b/packages/webapp/src/webview/ui/components/Header/SearchField/SearchField.tsx @@ -1,48 +1,102 @@ -import React from 'react'; +import React, { useCallback } from 'react'; +import debounce from 'lodash/debounce'; import { useSelector } from 'react-redux'; -import type { AppState } from '../../../../types'; -import { actions } from '../../../../state'; import { UISearchBox } from '@sap-ux/ui-components'; +import i18next from 'i18next'; +import type { AppState } from '../../../../types'; +import { actions } from '../../../../state'; import { Filters } from '../Filters'; -let timer: NodeJS.Timeout; +const SEARCH_TIMEOUT = 300; + /** + * SearchField component renders a search input field with debounce functionality. + * It interacts with the Redux state to manage search queries and filters. * - * @returns An input field + * @returns {React.JSX.Element} The rendered search field component. */ -export function SearchField() { +export const SearchField: React.FC = (): React.JSX.Element => { const appState = useSelector((state) => state); - const onChange = (value: string): void => { - clearTimeout(timer); - actions.setQueryValue(value); - timer = setTimeout(() => { - actions.searchTree({ - query: value, - filters: { - product: appState.selectedProductFilters, - component: appState.selectedComponentFilters - }, - paging: { - responseSize: appState.pageSize, - offset: 0 - } - }); - }, 100); + /** + * Fetches the search results for the given search term and filters. + * + * @param {string} value - The search term. + * @param {string[]} productFilters - The selected product filters. + * @param {string[]} componentFilter - The selected component filters. + * @param {number} pageSize - The number of results per page. + */ + const getTreesForSearchTerm = ( + value: string, + productFilters: string[], + componentFilter: string[], + pageSize: number + ): void => { + actions.searchTree({ + query: value, + filters: { + product: productFilters, + component: componentFilter + }, + paging: { + responseSize: pageSize, + offset: 0 + } + }); + }; + + /** + * Debounces the search input to avoid excessive API calls. + * + * @param {string} newSearchTerm - The new search term entered by the user. + * @param {string[]} productFilters - The selected product filters. + * @param {string[]} componentFilter - The selected component filters. + * @param {number} pageSize - The number of results per page. + */ + const debounceSearch = useCallback( + debounce( + (newSearchTerm: string, productFilters: string[], componentFilter: string[], pageSize: number) => + getTreesForSearchTerm(newSearchTerm, productFilters, componentFilter, pageSize), + SEARCH_TIMEOUT + ), + [] + ); + + /** + * Clears the search input and triggers a debounced search with empty values. + */ + const onClearInput = (): void => { + actions.setQueryValue(''); + debounceSearch('', appState.selectedProductFilters, appState.selectedComponentFilters, appState.pageSize); + }; + + /** + * Handles changes to the search input, updating the query value and triggering a debounced search. + * + * @param {React.ChangeEvent} [_] - The change event from the input field. + * @param {string} [newSearchTerm] - The new search term entered by the user. + */ + const onChangeInput = (_?: React.ChangeEvent | undefined, newSearchTerm: string = ''): void => { + actions.setQueryValue(newSearchTerm); + debounceSearch( + newSearchTerm, + appState.selectedProductFilters, + appState.selectedComponentFilters, + appState.pageSize + ); }; return ( -
+
onChange('')} - onChange={(e: any) => onChange(e?.target?.value || '')}> + onClear={onClearInput} + onChange={onChangeInput}> {appState.activeScreen === 'SEARCH' && }
); -} +}; diff --git a/packages/webapp/test/Header/SearchField.test.tsx b/packages/webapp/test/Header/SearchField.test.tsx index 5f46d201..ef9ce499 100644 --- a/packages/webapp/test/Header/SearchField.test.tsx +++ b/packages/webapp/test/Header/SearchField.test.tsx @@ -3,6 +3,8 @@ import React from 'react'; import { useSelector } from 'react-redux'; import { render, fireEvent, cleanup } from '@testing-library/react'; import { screen } from '@testing-library/dom'; +import { initIcons } from '@sap-ux/ui-components'; + import { SearchField } from '../../src/webview/ui/components/Header/SearchField'; import { initI18n } from '../../src/webview/i18n'; @@ -32,13 +34,14 @@ describe('', () => { productFilters: [], componentFilters: [] }, - query: 'fiori tools', + query: '', guideFeedback: true, selectedProductFilters: ['ProductFilter1, ProductFilter2'], selectedComponentFilters: ['ComponentFilter1', 'ComponentFilter2'], activeScreen: 'SEARCH' }; + initIcons(); initI18n(); afterEach(cleanup); @@ -60,4 +63,18 @@ describe('', () => { const { container } = render(); expect(container).toMatchSnapshot(); }); + + test('Should render a SearchField component, search value entered, clear icon is visible', async () => { + (useSelector as jest.Mock).mockImplementation((selector) => selector({ ...mockState, query: 'test' })); + render(); + + const searchInput = screen.getByRole('searchbox'); + const searchFieldContainerDOM = screen.getByTestId('search-field-container'); + let searchFieldContainerButtonsDOM = searchFieldContainerDOM?.querySelectorAll('button'); + + expect(searchFieldContainerButtonsDOM?.length).toEqual(3); + + fireEvent.click(searchFieldContainerButtonsDOM[0]); + expect(searchInput).toHaveFocus(); + }); }); diff --git a/packages/webapp/test/Header/__snapshots__/Header.test.tsx.snap b/packages/webapp/test/Header/__snapshots__/Header.test.tsx.snap index 9015f504..13b32faf 100644 --- a/packages/webapp/test/Header/__snapshots__/Header.test.tsx.snap +++ b/packages/webapp/test/Header/__snapshots__/Header.test.tsx.snap @@ -154,6 +154,7 @@ exports[`
Should render a Header component without the navigation butt
Should render a Header component without the navigation butt class="ms-SearchBox-field field-113" id="search-field" placeholder="Search Guided Answers" - readonly="" role="searchbox" value="" /> diff --git a/packages/webapp/test/Header/__snapshots__/SearchField.test.tsx.snap b/packages/webapp/test/Header/__snapshots__/SearchField.test.tsx.snap index df1ce2a8..8c6f321f 100644 --- a/packages/webapp/test/Header/__snapshots__/SearchField.test.tsx.snap +++ b/packages/webapp/test/Header/__snapshots__/SearchField.test.tsx.snap @@ -4,9 +4,10 @@ exports[` Should render a SearchField component, on home screen 1
-
-
-
-