Skip to content

Commit

Permalink
style: Pass at propagating (and enhancing) Button component throughou…
Browse files Browse the repository at this point in the history
…t Superset (#10649)

* getting rid of weird focus/active outline ring

* Buttons... buttons _everywhere_

* linting

* Nixing views/CRUD/dataset/Button component

* fixing 2 typing errors

* fixing more TS errors

* prefer src path for include

* one more real button, one less CSS class

* one more "button" to "Button"

* Published Status is now a proper clickable Label

* nixing the CRUD button again

* touching up stories, with SupersetButton story

* SIP-34 button colors

* adding polished package to mix colors

* updating button colors to match Superset theme

* abstracting away from bootstrap-specific props (might pivot libraries soon!)

* more abstraction from bsStyle/bsSize props

* exchanging styles for a prop

* linting

* restoring feature flag to stock

* using src alias

* last <button> replacement

* this classname would never be applied

* more linting action

* fixing unsupported bsSize 'medium', and cta typing error

* more cta action

* unnecessary styles

* errant bsSize prop

* cleanup

* tweaks to make new New button work

* Linting

* fixing a couple tests

* fixing theme based test failure

* margin tweak for NEW button

* another fixed test

* another fixed test

* fixing two more tests

* fixing last broken tests.

* always be linting

* Adding tertiary/dashed buttons

* cleaning up QueryAndSave buttons

* fixing "link" button styles

* fixing/updating link button styles

* cta buttons on Modal component

* linting.

* exporting button story knobs, making ALL knobs safe for export.

* capitalizing a file... no big whoop

* Basic button tests

* renaming button - temporarily

* renaming file to fix capitalization issue

* passing theme through to a difficult popover.

* fixin' a newly busted unit test

* lint fixin'

* oops, shouldn't have changed this prop!

* adding a dive() to themedShallow, and fixing a cypress/jest test

* addressing lint stuff

* touching up stories, with SupersetButton story

* SIP-34 button colors

* updating button colors to match Superset theme

* abstracting away from bootstrap-specific props (might pivot libraries soon!)

* linting

* restoring feature flag to stock

* cleanup

* Linting

* renaming button - temporarily

* renaming file to fix capitalization issue

* oops, shouldn't have changed this prop!

* adding a dive() to themedShallow, and fixing a cypress/jest test

* addressing lint stuff

* nixing new modal button

* Fixing another popover/button issue that should break cypress

* lint ✨

* passing classNames through to new button (should fix some tests)

* cleaning unused classes, making cypress tests use data attrs

* fixin' the test

* fixing another class-based test with data-test attr

* no longer passing theme as prop to buttons in popovers... themeprovider is better

* outline/border tweaks!
  • Loading branch information
rusackas authored Aug 29, 2020
1 parent 33fa9eb commit 9fe30ab
Show file tree
Hide file tree
Showing 78 changed files with 755 additions and 557 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ describe('AdhocFilters', () => {
cy.get('button').contains('Save').click();
});

cy.get('button.query').click();
cy.get('button[data-test="run-query-button"]').click();
cy.verifySliceSuccess({
waitAlias: '@postJson',
chartSelector: 'svg',
Expand All @@ -63,7 +63,7 @@ describe('AdhocFilters', () => {
cy.get('button').contains('Save').click();
});

cy.get('button.query').click();
cy.get('button[data-test="run-query-button"]').click();
cy.verifySliceSuccess({
waitAlias: '@postJson',
chartSelector: 'svg',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ describe('AdhocMetrics', () => {

cy.get('.metrics-select .metric-option').contains(metricName);

cy.get('button.query').click();
cy.get('button[data-test="run-query-button"]').click();
cy.verifySliceSuccess({
waitAlias: '@postJson',
querySubstring: `${metric} AS "${metricName}"`, // SQL statement
Expand Down Expand Up @@ -77,7 +77,7 @@ describe('AdhocMetrics', () => {
.type('/COUNT(DISTINCT name)', { force: true });
cy.get('#metrics-edit-popover').find('button').contains('Save').click();

cy.get('button.query').click();
cy.get('button[data-test="run-query-button"]').click();

const metric = 'SUM(num)/COUNT(DISTINCT name)';
cy.verifySliceSuccess({
Expand All @@ -103,7 +103,7 @@ describe('AdhocMetrics', () => {
cy.get('button').contains('Save').click();
});

cy.get('button.query').click();
cy.get('button[data-test="run-query-button"]').click();

const metric = 'SUM(num)';
cy.verifySliceSuccess({
Expand All @@ -124,7 +124,7 @@ describe('AdhocMetrics', () => {
});

const metric = 'AVG(sum_boys)';
cy.get('button.query').click();
cy.get('button[data-test="run-query-button"]').click();
cy.verifySliceSuccess({
waitAlias: '@postJson',
querySubstring: `${metric} AS "${metric}"`,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ describe('Advanced analytics', () => {
.find('.Select__multi-value__label')
.contains('364 days');

cy.get('button.query').click();
cy.get('button[data-test="run-query-button"]').click();
cy.wait('@postJson');
cy.reload();
cy.verifySliceSuccess({
Expand Down Expand Up @@ -90,7 +90,7 @@ describe('Annotations', () => {
cy.get('button').contains('OK').click();
});

cy.get('button.query').click();
cy.get('button[data-test="run-query-button"]').click();
cy.verifySliceSuccess({
waitAlias: '@postJson',
chartSelector: 'svg',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ describe('Groupby control', () => {
cy.get('.Select__control').click();
cy.get('input[type=text]').type('state{enter}');
});
cy.get('button.query').click();
cy.get('button[data-test="run-query-button"]').click();
cy.verifySliceSuccess({ waitAlias: '@postJson', chartSelector: 'svg' });
});
});
Expand Down
8 changes: 3 additions & 5 deletions superset-frontend/package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions superset-frontend/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,7 @@
"mousetrap": "^1.6.1",
"mustache": "^2.2.1",
"omnibar": "^2.1.1",
"polished": "^3.6.5",
"prop-types": "^15.7.2",
"re-resizable": "^4.3.1",
"react": "^16.13.1",
Expand Down
2 changes: 1 addition & 1 deletion superset-frontend/spec/helpers/theming.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,5 +51,5 @@ export function styledShallow(
theme: supersetTheme,
...options?.wrappingComponentProps,
},
});
}).dive();
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
*/
import React from 'react';
import { shallow, ShallowWrapper } from 'enzyme';
import { Button } from 'react-bootstrap';
import Button from 'src/components/Button';
import Select from 'src/components/Select';
import AddSliceContainer, {
AddSliceContainerProps,
Expand Down Expand Up @@ -58,9 +58,9 @@ describe('AddSliceContainer', () => {
});

it('renders a disabled button if no datasource is selected', () => {
expect(
wrapper.find(Button).dive().find('.btn[disabled=true]'),
).toHaveLength(1);
expect(wrapper.find(Button).dive().find({ disabled: true })).toHaveLength(
1,
);
});

it('renders an enabled button if datasource is selected', () => {
Expand All @@ -70,9 +70,9 @@ describe('AddSliceContainer', () => {
datasourceId: datasourceValue.split('__')[0],
datasourceType: datasourceValue.split('__')[1],
});
expect(
wrapper.find(Button).dive().find('.btn[disabled=false]'),
).toHaveLength(1);
expect(wrapper.find(Button).dive().find({ disabled: true })).toHaveLength(
0,
);
});

it('formats explore url', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
*/
import React from 'react';
import { mount } from 'enzyme';
import { Button } from 'react-bootstrap';
import Button from 'src/components/Button';
import { supersetTheme, ThemeProvider } from '@superset-ui/style';
import ConfirmStatusChange from 'src/components/ConfirmStatusChange';
import Modal from 'src/components/Modal';
Expand All @@ -33,7 +33,7 @@ describe('ConfirmStatusChange', () => {
<ConfirmStatusChange {...mockedProps}>
{confirm => (
<>
<button id="btn1" onClick={confirm} />
<Button id="btn1" onClick={confirm} />
</>
)}
</ConfirmStatusChange>,
Expand All @@ -44,7 +44,7 @@ describe('ConfirmStatusChange', () => {
);

it('opens a confirm modal', () => {
wrapper.find('#btn1').props().onClick('foo');
wrapper.find('#btn1').first().props().onClick('foo');

wrapper.update();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
*/
import { Provider } from 'react-redux';
import React from 'react';
import { mount } from 'enzyme';
import { styledMount as mount } from 'spec/helpers/theming';
import sinon from 'sinon';

import DashboardComponent from 'src/dashboard/containers/DashboardComponent';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,10 @@ describe('DatasourceModal', () => {

it('saves on confirm', async () => {
act(() => {
wrapper.find('[className="m-r-5"]').props().onClick();
wrapper
.find('button[data-test="datasource-modal-save"]')
.props()
.onClick();
});
await waitForComponentToPaint(wrapper);
act(() => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,8 @@
import React from 'react';
import sinon from 'sinon';
import { shallow } from 'enzyme';
import { Button, Popover, Tab, Tabs } from 'react-bootstrap';
import { Popover, Tab, Tabs } from 'react-bootstrap';
import Button from 'src/components/Button';

import AdhocFilter, {
EXPRESSION_TYPES,
Expand Down Expand Up @@ -117,9 +118,9 @@ describe('AdhocFilterEditPopover', () => {

it('highlights save if changes are present', () => {
const { wrapper } = setup();
expect(wrapper.find(Button).find({ bsStyle: 'primary' })).not.toExist();
expect(wrapper.find(Button).find({ buttonStyle: 'primary' })).not.toExist();
wrapper.instance().onAdhocFilterChange(sqlAdhocFilter);
expect(wrapper.find(Button).find({ bsStyle: 'primary' })).toExist();
expect(wrapper.find(Button).find({ buttonStyle: 'primary' })).toExist();
});

it('will initiate a drag when clicked', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
/* eslint-disable no-unused-expressions */
import React from 'react';
import sinon from 'sinon';
import { shallow } from 'enzyme';
import { styledShallow as shallow } from 'spec/helpers/theming';
import { OverlayTrigger } from 'react-bootstrap';

import Label from 'src/components/Label';
Expand All @@ -46,7 +46,7 @@ function setup(overrides) {
datasource: {},
...overrides,
};
const wrapper = shallow(<AdhocFilterOption {...props} />);
const wrapper = shallow(<AdhocFilterOption {...props} />).dive();
return { wrapper };
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,8 @@
import React from 'react';
import sinon from 'sinon';
import { shallow } from 'enzyme';
import { Button, FormGroup, Popover } from 'react-bootstrap';
import { FormGroup, Popover } from 'react-bootstrap';
import Button from 'src/components/Button';

import AdhocMetric, { EXPRESSION_TYPES } from 'src/explore/AdhocMetric';
import AdhocMetricEditPopover from 'src/explore/components/AdhocMetricEditPopover';
Expand Down Expand Up @@ -117,9 +118,9 @@ describe('AdhocMetricEditPopover', () => {

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

it('will initiate a drag when clicked', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
/* eslint-disable no-unused-expressions */
import React from 'react';
import sinon from 'sinon';
import { shallow } from 'enzyme';
import { styledShallow as shallow } from 'spec/helpers/theming';
import { OverlayTrigger } from 'react-bootstrap';

import Label from 'src/components/Label';
Expand All @@ -46,7 +46,7 @@ function setup(overrides) {
columns,
...overrides,
};
const wrapper = shallow(<AdhocMetricOption {...props} />);
const wrapper = shallow(<AdhocMetricOption {...props} />).dive();
return { wrapper, onMetricEdit };
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
import React from 'react';
import sinon from 'sinon';
import { styledMount as mount } from 'spec/helpers/theming';
import { Button } from 'react-bootstrap';
import Button from 'src/components/Button';

import Label from 'src/components/Label';
import DateFilterControl from 'src/explore/components/controls/DateFilterControl';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ describe('QueryAndSaveButtons', () => {
});

it('calls onQuery when query button is clicked', () => {
const queryButton = wrapper.find('.query');
const queryButton = wrapper.find('[data-test="run-query-button"]');
queryButton.simulate('click');
expect(defaultProps.onQuery.called).toBe(true);
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,10 @@ import configureStore from 'redux-mock-store';
import thunk from 'redux-thunk';
import { bindActionCreators } from 'redux';

import { shallow, mount } from 'enzyme';
import { FormControl, Modal, Button, Radio } from 'react-bootstrap';
import { shallow } from 'enzyme';
import { styledMount as mount } from 'spec/helpers/theming';
import { FormControl, Modal, Radio } from 'react-bootstrap';
import Button from 'src/components/Button';
import sinon from 'sinon';
import fetchMock from 'fetch-mock';

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
* under the License.
*/
import React from 'react';
import { Button } from 'react-bootstrap';
import Button from 'src/components/Button';
import { shallow } from 'enzyme';
import sinon from 'sinon';

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ describe('SouthPane', () => {
const getWrapper = () =>
shallow(<SouthPaneContainer {...mockedProps} />, {
context: { store },
}).dive();
});

let wrapper;

Expand Down
4 changes: 2 additions & 2 deletions superset-frontend/src/CRUD/CollectionTable.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
import React, { ReactNode } from 'react';
import shortid from 'shortid';
import { t } from '@superset-ui/translation';
import Button from '../components/Button';
import Button from 'src/components/Button';
import Fieldset from './Fieldset';
import { recurseReactClone } from './utils';
import './crud.less';
Expand Down Expand Up @@ -167,7 +167,7 @@ export default class CRUDCollection extends React.PureComponent<
{allowDeletes && !allowAddItem && <th className="tiny-cell" />}
{allowAddItem && (
<th>
<Button bsStyle="primary" onClick={this.onAddItem}>
<Button buttonStyle="primary" onClick={this.onAddItem}>
<i className="fa fa-plus" /> {t('Add Item')}
</Button>
</th>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import { Table } from 'reactable-arc';
import { Alert } from 'react-bootstrap';
import { t } from '@superset-ui/translation';

import Button from '../../components/Button';
import Button from 'src/components/Button';
import Loading from '../../components/Loading';
import ModalTrigger from '../../components/ModalTrigger';

Expand Down Expand Up @@ -85,8 +85,8 @@ class EstimateQueryCostButton extends React.PureComponent {
modalBody={this.renderModalBody()}
triggerNode={
<Button
bsStyle="warning"
bsSize="small"
buttonStyle="warning"
buttonSize="small"
onClick={this.onClick}
key="query-estimate-btn"
tooltip={tooltip}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,9 @@ import Dialog from 'react-bootstrap-dialog';
import { t } from '@superset-ui/translation';
import { InfoTooltipWithTrigger } from '@superset-ui/chart-controls';

import Button from 'src/components/Button';
import { exploreChart } from '../../explore/exploreUtils';
import * as actions from '../actions/sqlLab';
import Button from '../../components/Button';

const propTypes = {
actions: PropTypes.object.isRequired,
Expand Down Expand Up @@ -89,7 +89,7 @@ class ExploreCtasResultsButton extends React.PureComponent {
return (
<>
<Button
bsSize="small"
buttonSize="small"
onClick={this.onClick}
tooltip={t('Explore the result set in the data exploration view')}
>
Expand Down
Loading

0 comments on commit 9fe30ab

Please sign in to comment.