Skip to content

Commit

Permalink
feat(explore-popover): Show disabled 'Save' button in explore popover (
Browse files Browse the repository at this point in the history
…#21318)

Co-authored-by: Herbert Gainor <herbert.gainor@preset.io>
Co-authored-by: Michael S. Molina <michael.s.molina@gmail.com>
  • Loading branch information
3 people authored Dec 7, 2022
1 parent 2731cba commit 0dbaaad
Show file tree
Hide file tree
Showing 6 changed files with 40 additions and 31 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -406,10 +406,8 @@ const ColumnSelectPopover = ({
{t('Close')}
</Button>
<Button
disabled={!stateIsValid}
buttonStyle={
hasUnsavedChanges && stateIsValid ? 'primary' : 'default'
}
disabled={!stateIsValid || !hasUnsavedChanges}
buttonStyle="primary"
buttonSize="small"
onClick={onSave}
data-test="ColumnEdit#save"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ describe('AdhocFilterEditPopover', () => {

it('prevents saving if the filter is invalid', () => {
const { wrapper } = setup();
expect(wrapper.find(Button).find({ disabled: true })).not.toExist();
expect(wrapper.find(Button).find({ disabled: true })).toExist();
wrapper
.instance()
.onAdhocFilterChange(simpleAdhocFilter.duplicateWith({ operator: null }));
Expand All @@ -133,7 +133,6 @@ describe('AdhocFilterEditPopover', () => {

it('highlights save if changes are present', () => {
const { wrapper } = setup();
expect(wrapper.find(Button).find({ buttonStyle: 'primary' })).not.toExist();
wrapper.instance().onAdhocFilterChange(sqlAdhocFilter);
expect(wrapper.find(Button).find({ buttonStyle: 'primary' })).toExist();
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -245,10 +245,12 @@ export default class AdhocFilterEditPopover extends React.Component {
</Button>
<Button
data-test="adhoc-filter-edit-popover-save-button"
disabled={!stateIsValid || !this.state.isSimpleTabValid}
buttonStyle={
hasUnsavedChanges && stateIsValid ? 'primary' : 'default'
disabled={
!stateIsValid ||
!this.state.isSimpleTabValid ||
!hasUnsavedChanges
}
buttonStyle="primary"
buttonSize="small"
className="m-r-5"
onClick={this.onSave}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ describe('AdhocMetricEditPopover', () => {

it('prevents saving if no column or aggregate is chosen', () => {
const { wrapper } = setup();
expect(wrapper.find(Button).find({ disabled: true })).not.toExist();
expect(wrapper.find(Button).find({ disabled: false })).not.toExist();
wrapper.instance().onColumnChange(null);
expect(wrapper.find(Button).find({ disabled: true })).toExist();
wrapper.instance().onColumnChange(columns[0].column_name);
Expand All @@ -109,9 +109,9 @@ describe('AdhocMetricEditPopover', () => {

it('highlights save if changes are present', () => {
const { wrapper } = setup();
expect(wrapper.find(Button).find({ buttonStyle: 'primary' })).not.toExist();
expect(wrapper.find(Button).find({ disabled: true })).toExist();
wrapper.instance().onColumnChange(columns[1].column_name);
expect(wrapper.find(Button).find({ buttonStyle: 'primary' })).toExist();
expect(wrapper.find(Button).find({ disabled: true })).not.toExist();
});

it('will initiate a drag when clicked', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
*/
import userEvent from '@testing-library/user-event';
import React from 'react';
import { render, screen } from 'spec/helpers/testing-library';
import { render, screen, waitFor, within } from 'spec/helpers/testing-library';
import AdhocMetric from 'src/explore/components/controls/MetricControl/AdhocMetric';
import AdhocMetricEditPopover from '.';

Expand All @@ -35,10 +35,15 @@ const createProps = () => ({
},
savedMetricsOptions: [
{
id: 65,
id: 64,
metric_name: 'count',
expression: 'COUNT(*)',
},
{
id: 65,
metric_name: 'sum',
expression: 'sum(num)',
},
],
adhocMetric: new AdhocMetric({ isNew: true }),
datasource: {
Expand Down Expand Up @@ -118,28 +123,35 @@ test('Clicking on "Close" should call onClose', () => {
expect(props.onClose).toBeCalledTimes(1);
});

test('Clicking on "Save" should call onChange and onClose', () => {
test('Clicking on "Save" should call onChange and onClose', async () => {
const props = createProps();
render(<AdhocMetricEditPopover {...props} />);
expect(props.onChange).toBeCalledTimes(0);
expect(props.onClose).toBeCalledTimes(0);
userEvent.click(
screen.getByRole('combobox', {
name: 'Select saved metrics',
}),
);
const sumOption = await waitFor(() =>
within(document.querySelector('.rc-virtual-list')!).getByText('sum'),
);
userEvent.click(sumOption);
userEvent.click(screen.getByRole('button', { name: 'Save' }));
expect(props.onChange).toBeCalledTimes(1);
expect(props.onChange).toBeCalledWith(
{
id: 64,
metric_name: 'count',
expression: 'COUNT(*)',
},
{
id: 64,
metric_name: 'count',
expression: 'COUNT(*)',
},
);
expect(props.onClose).toBeCalledTimes(1);
});

test('Clicking on "Save" should not call onChange and onClose', () => {
const props = createProps();
render(<AdhocMetricEditPopover {...props} />);
expect(props.onChange).toBeCalledTimes(0);
expect(props.onClose).toBeCalledTimes(0);
userEvent.click(screen.getByRole('button', { name: 'Save' }));
expect(props.onChange).toBeCalledTimes(0);
expect(props.onClose).toBeCalledTimes(0);
});

test('Should switch to tab:Simple', () => {
const props = createProps();
props.getCurrentTab.mockImplementation(tab => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -487,10 +487,8 @@ export default class AdhocMetricEditPopover extends React.PureComponent {
{t('Close')}
</Button>
<Button
disabled={!stateIsValid}
buttonStyle={
hasUnsavedChanges && stateIsValid ? 'primary' : 'default'
}
disabled={!stateIsValid || !hasUnsavedChanges}
buttonStyle="primary"
buttonSize="small"
data-test="AdhocMetricEdit#save"
onClick={this.onSave}
Expand Down

0 comments on commit 0dbaaad

Please sign in to comment.