Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: remove loading indicator when typing in select #18799

Merged
merged 10 commits into from
Mar 3, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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