Skip to content

Commit

Permalink
chore: Select component refactoring - SaveModal - Iteration 5 (#16446)
Browse files Browse the repository at this point in the history
* Refactor Select

* Fix Cypress

* Reconcile with master

* Use onChange over onSelect

* Reconcile with latest changes

* Update Cypress

* Update Cypress test

* Fix lint
  • Loading branch information
geido committed Sep 27, 2021
1 parent ab9f8cb commit 271ec6e
Show file tree
Hide file tree
Showing 5 changed files with 28 additions and 24 deletions.
2 changes: 1 addition & 1 deletion CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -805,7 +805,7 @@ npm install
npm run cypress-run-chrome

# run tests from a specific file
npm run cypress-run-chrome -- --spec cypress/integration/explore/link.test.js
npm run cypress-run-chrome -- --spec cypress/integration/explore/link.test.ts

# run specific file with video capture
npm run cypress-run-chrome -- --spec cypress/integration/dashboard/index.test.js --config video=true
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,8 @@ xdescribe('Nativefilters', () => {
.click();
cy.get('[data-test="query-save-button"]').click();
cy.get('[data-test="save-chart-modal-select-dashboard-form"]')
.find('#dashboard-creatable-select')
.type(`${dashboard}{enter}{enter}`);
.find('input[aria-label="Select a dashboard"]')
.type(`${dashboard}`, { force: true });
cy.get('[data-test="btn-modal-save"]').click();
});
beforeEach(() => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -126,8 +126,12 @@ describe('Test explore links', () => {
cy.get('[data-test="new-chart-name"]').click().clear().type(newChartName);
// Add a new option using the "CreatableSelect" feature
cy.get('[data-test="save-chart-modal-select-dashboard-form"]')
.find('#dashboard-creatable-select')
.type(`${dashboardTitle}{enter}{enter}`);
.find('input[aria-label="Select a dashboard"]')
.type(`${dashboardTitle}`, { force: true });

cy.get(`.ant-select-item[label="${dashboardTitle}"]`).click({
force: true,
});

cy.get('[data-test="btn-modal-save"]').click();
cy.verifySliceSuccess({ waitAlias: '@chartData' });
Expand All @@ -153,8 +157,12 @@ describe('Test explore links', () => {
// This time around, typing the same dashboard name
// will select the existing one
cy.get('[data-test="save-chart-modal-select-dashboard-form"]')
.find('#dashboard-creatable-select')
.type(`${dashboardTitle}{enter}{enter}`);
.find('input[aria-label="Select a dashboard"]')
.type(`${dashboardTitle}{enter}`, { force: true });

cy.get(`.ant-select-item[label="${dashboardTitle}"]`).click({
force: true,
});

cy.get('[data-test="btn-modal-save"]').click();
cy.verifySliceSuccess({ waitAlias: '@chartData' });
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -142,12 +142,13 @@ describe('SaveModal', () => {

it('onChange', () => {
const wrapper = getWrapper();
const dashboardId = mockEvent.value;

wrapper.instance().onSliceNameChange(mockEvent);
expect(wrapper.state().newSliceName).toBe(mockEvent.target.value);

wrapper.instance().onDashboardSelectChange(mockEvent);
expect(wrapper.state().saveToDashboardId).toBe(mockEvent.value);
wrapper.instance().onDashboardSelectChange(dashboardId);
expect(wrapper.state().saveToDashboardId).toBe(dashboardId);
});

describe('saveOrOverwrite', () => {
Expand Down
25 changes: 10 additions & 15 deletions superset-frontend/src/explore/components/SaveModal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,8 @@ import ReactMarkdown from 'react-markdown';
import Modal from 'src/components/Modal';
import { Radio } from 'src/components/Radio';
import Button from 'src/components/Button';
import { CreatableSelect } from 'src/components/Select';
import { Select } from 'src/components';
import { SelectValue } from 'antd/lib/select';
import { connect } from 'react-redux';

// Session storage key for recent dashboard
Expand Down Expand Up @@ -108,10 +109,10 @@ class SaveModal extends React.Component<SaveModalProps, SaveModalState> {
this.setState({ newSliceName: event.target.value });
}

onDashboardSelectChange(event: Record<string, any>) {
const newDashboardName = event ? event.label : null;
onDashboardSelectChange(selected: SelectValue) {
const newDashboardName = selected ? String(selected) : undefined;
const saveToDashboardId =
event && typeof event.value === 'number' ? event.value : null;
selected && typeof selected === 'number' ? selected : null;
this.setState({ saveToDashboardId, newDashboardName });
}

Expand Down Expand Up @@ -163,9 +164,6 @@ class SaveModal extends React.Component<SaveModalProps, SaveModalState> {
render() {
const dashboardSelectValue =
this.state.saveToDashboardId || this.state.newDashboardName;
const valueObj = dashboardSelectValue
? { value: dashboardSelectValue }
: null;
return (
<StyledModal
show
Expand Down Expand Up @@ -260,16 +258,13 @@ class SaveModal extends React.Component<SaveModalProps, SaveModalState> {
label={t('Add to dashboard')}
data-test="save-chart-modal-select-dashboard-form"
>
<CreatableSelect
id="dashboard-creatable-select"
className="save-modal-selector"
menuPosition="fixed"
<Select
allowClear
allowNewOptions
ariaLabel={t('Select a dashboard')}
options={this.props.dashboards}
clearable
creatable
onChange={this.onDashboardSelectChange}
autoSize={false}
value={valueObj}
value={dashboardSelectValue || undefined}
placeholder={
// Using markdown to allow for good i18n
<ReactMarkdown
Expand Down

0 comments on commit 271ec6e

Please sign in to comment.