Skip to content

Commit

Permalink
feat: remove loading indicator when typing in select (#18799)
Browse files Browse the repository at this point in the history
(cherry picked from commit 5a8eb09)
  • Loading branch information
ktmud authored and villebro committed Apr 3, 2022
1 parent 75afb3a commit 78e85ae
Show file tree
Hide file tree
Showing 4 changed files with 126 additions and 119 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -127,21 +127,10 @@ describe('Nativefilters Sanity test', () => {
.within(() =>
cy.get('input').type('wb_health_population{enter}', { force: true }),
);
// Add following step to avoid flaky enter value in line 177
cy.get(nativeFilters.filtersPanel.inputDropdown)
.should('be.visible', { timeout: 20000 })
.last()
.click();

cy.get('.loading inline-centered css-101mkpk').should('not.exist');
// hack for unclickable country_name
cy.wait(5000);
cy.get(nativeFilters.filtersPanel.filterInfoInput)
.last()
.should('be.visible', { timeout: 30000 })
.click({ force: true });
cy.get(nativeFilters.filtersPanel.filterInfoInput)
cy.get(`${nativeFilters.filtersPanel.filterInfoInput}:visible:last`)
.last()
.focus()
.type('country_name');
cy.get(nativeFilters.filtersPanel.inputDropdown)
.should('be.visible', { timeout: 20000 })
Expand Down
62 changes: 25 additions & 37 deletions superset-frontend/src/components/Select/Select.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,7 @@
* under the License.
*/
import React from 'react';
import {
render,
screen,
waitFor,
waitForElementToBeRemoved,
within,
} from 'spec/helpers/testing-library';
import { render, screen, waitFor, within } from 'spec/helpers/testing-library';
import userEvent from '@testing-library/user-event';
import { Select } from 'src/components';

Expand Down Expand Up @@ -388,13 +382,6 @@ test('static - does not show "Loading..." when allowNewOptions is false and a ne
expect(screen.queryByText(LOADING)).not.toBeInTheDocument();
});

test('static - shows "Loading..." when allowNewOptions is true and a new option is entered', async () => {
render(<Select {...defaultProps} allowNewOptions />);
await open();
await type(NEW_OPTION);
expect(await screen.findByText(LOADING)).toBeInTheDocument();
});

test('static - does not add a new option if the option already exists', async () => {
render(<Select {...defaultProps} allowNewOptions />);
const option = OPTIONS[0].label;
Expand Down Expand Up @@ -456,15 +443,6 @@ test('async - displays the loading indicator when opening', async () => {
expect(screen.queryByText(LOADING)).not.toBeInTheDocument();
});

test('async - displays the loading indicator while searching', async () => {
render(<Select {...defaultProps} options={loadOptions} />);
await type('John');
expect(screen.getByText(LOADING)).toBeInTheDocument();
await waitFor(() =>
expect(screen.queryByText(LOADING)).not.toBeInTheDocument(),
);
});

test('async - makes a selection in single mode', async () => {
render(<Select {...defaultProps} options={loadOptions} />);
const optionText = 'Emma';
Expand Down Expand Up @@ -587,24 +565,29 @@ test('async - sets a initial value in multiple mode', async () => {
expect(values[1]).toHaveTextContent(OPTIONS[1].label);
});

test('async - searches for an item already loaded', async () => {
test('async - searches for matches in both loaded and unloaded pages', async () => {
render(<Select {...defaultProps} options={loadOptions} />);
const search = 'Oli';
await open();
await type(search);
await waitForElementToBeRemoved(screen.getByText(LOADING));
const options = await findAllSelectOptions();
await type('and');

let options = await findAllSelectOptions();
expect(options.length).toBe(1);
expect(options[0]).toHaveTextContent('Alehandro');

await screen.findByText('Sandro');
options = await findAllSelectOptions();
expect(options.length).toBe(2);
expect(options[0]).toHaveTextContent('Oliver');
expect(options[1]).toHaveTextContent('Olivia');
expect(options[0]).toHaveTextContent('Alehandro');
expect(options[1]).toHaveTextContent('Sandro');
});

test('async - searches for an item in a page not loaded', async () => {
render(<Select {...defaultProps} options={loadOptions} />);
const search = 'Ashfaq';
const mock = jest.fn(loadOptions);
render(<Select {...defaultProps} options={mock} />);
const search = 'Sandro';
await open();
await type(search);
await waitForElementToBeRemoved(screen.getByText(LOADING));
await waitFor(() => expect(mock).toHaveBeenCalledTimes(2));
const options = await findAllSelectOptions();
expect(options.length).toBe(1);
expect(options[0]).toHaveTextContent(search);
Expand Down Expand Up @@ -650,7 +633,7 @@ test('async - does not fire a new request for the same search input', async () =
expect(loadOptions).toHaveBeenCalledTimes(1);
clearAll();
await type('search');
expect(await screen.findByText(NO_DATA)).toBeInTheDocument();
expect(await screen.findByText(LOADING)).toBeInTheDocument();
expect(loadOptions).toHaveBeenCalledTimes(1);
});

Expand All @@ -668,13 +651,18 @@ test('async - does not fire a new request if all values have been fetched', asyn

test('async - fires a new request if all values have not been fetched', async () => {
const mock = jest.fn(loadOptions);
const search = 'George';
const pageSize = OPTIONS.length / 2;
render(<Select {...defaultProps} options={mock} pageSize={pageSize} />);
await open();
expect(mock).toHaveBeenCalledTimes(1);
await type(search);
expect(await findSelectOption(search)).toBeInTheDocument();
await type('or');

// `George` is on the first page so when it appears the API has not been called again
expect(await findSelectOption('George')).toBeInTheDocument();
expect(mock).toHaveBeenCalledTimes(1);

// `Igor` is on the second paged API request
expect(await findSelectOption('Igor')).toBeInTheDocument();
expect(mock).toHaveBeenCalledTimes(2);
});

Expand Down
Loading

0 comments on commit 78e85ae

Please sign in to comment.