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.

Sign up for GitHub

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

Jump to bottom

style: DOCTYPE tag, and related CSS cleanup/refactoring #10302

Merged
merged 17 commits into from Jul 30, 2020
Merged

style: DOCTYPE tag, and related CSS cleanup/refactoring #10302

Show file tree
Hide file tree
Changes from 16 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
671a1b4
style: Adds DOCTYPE, Emotion wrappers, flexbox layout improvements
rusackas Jul 13, 2020
1627074
fix: unused import, linting
rusackas Jul 13, 2020
eafff37
nixing unused imports
rusackas Jul 22, 2020
626fd48
Fix bug causing infinite rerender of save modal in chart.
Jul 23, 2020
f7e710a
changing a selector to pass a cypress test
rusackas Jul 23, 2020
dcb56b6
Update cyp test calls.
Jul 28, 2020
5061f7b
Fix tests for adhoc metrics.
Jul 28, 2020
4ec06bc
Add more e2e test fixes.
Jul 28, 2020
e61eb18
Fix final cypress test.
Jul 28, 2020
9825574
Move emotion definition out of render method.
Jul 29, 2020
fc806a8
fixing control pane scroll issue
rusackas Jul 29, 2020
1f9e961
controls scroll WITHIN the control tab
rusackas Jul 29, 2020
d62975a
fixing panel bg
rusackas Jul 29, 2020
f3ec1b0
circle buttons are circles.
rusackas Jul 29, 2020
f056bae
re-fixing tables, including padding on the left
rusackas Jul 29, 2020
e0e15da
fix cypress spec
nytai Jul 30, 2020
12e5b98
unsquish license comment
nytai Jul 30, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
55 changes: 29 additions & 26 deletions superset-frontend/cypress-base/cypress/integration/explore/AdhocMetrics.test.ts
View file
Open in desktop
Expand Up @@ -26,25 +26,27 @@ describe('AdhocMetrics', () => {

it('Clear metric and set simple adhoc metric', () => {
const metric = 'sum(sum_girls)';
const metricName = 'Girl Births';
const metricName = 'Sum Girls';

cy.visitChartByName('Num Births Trend');
cy.verifySliceSuccess({ waitAlias: '@postJson' });

cy.get('[data-test=metrics]').within(() => {
cy.get('.Select__clear-indicator').click();
cy.get('.Select__control input').type('sum_girls');
cy.get('.Select__option--is-focused').trigger('mousedown').click();
});
cy.get('[data-test=metrics]').find('.Select__clear-indicator').click();

cy.get('#metrics-edit-popover').within(() => {
cy.get('.popover-title').within(() => {
cy.get('span').click();
cy.get('input').type(metricName);
});
cy.get('button').contains('Save').click();
});
cy.get('.Select__multi-value__label').contains(metricName);
cy.get('[data-test=metrics]')
.find('.Select__control input')
.type('sum_girls', { force: true });

cy.get('[data-test=metrics]')
.find('.Select__option--is-focused')
.trigger('mousedown')
.click();

cy.get('[data-test="AdhocMetricEditTitle#trigger"]').click();
cy.get('[data-test="AdhocMetricEditTitle#input"]').type(metricName);
cy.get('[data-test="AdhocMetricEdit#save"]').contains('Save').click();

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

cy.get('button.query').click();
cy.verifySliceSuccess({
Expand All @@ -59,20 +61,21 @@ describe('AdhocMetrics', () => {
cy.verifySliceSuccess({ waitAlias: '@postJson' });

// select column "num"
cy.get('[data-test=metrics]').within(() => {
cy.get('.Select__clear-indicator').click();
cy.get('.Select__control').click();
cy.get('.Select__control input').type('num');
cy.get('.option-label').contains(/^num$/).click();
});
cy.get('[data-test=metrics]').find('.Select__clear-indicator').click();

cy.get('[data-test=metrics]').find('.Select__control').click();

cy.get('[data-test=metrics]').find('.Select__control input').type('num');

cy.get('[data-test=metrics]').find('.option-label').last().click();

// add custom SQL
cy.get('#metrics-edit-popover').within(() => {
cy.get('#adhoc-metric-edit-tabs-tab-SQL').click();
cy.get('.ace_content').click();
cy.get('.ace_text-input').type('/COUNT(DISTINCT name)', { force: true });
cy.get('button').contains('Save').click();
});
cy.get('#adhoc-metric-edit-tabs-tab-SQL').click();
cy.get('#metrics-edit-popover').find('.ace_content').click();
cy.get('#metrics-edit-popover')
.find('.ace_text-input')
.type('/COUNT(DISTINCT name)', { force: true });
cy.get('#metrics-edit-popover').find('button').contains('Save').click();

cy.get('button.query').click();

Expand Down
18 changes: 11 additions & 7 deletions superset-frontend/cypress-base/cypress/integration/explore/advanced.test.ts
View file
Open in desktop
Expand Up @@ -30,14 +30,18 @@ describe('Advanced analytics', () => {

cy.get('.panel-title').contains('Advanced Analytics').click();

cy.get('[data-test=time_compare]').within(() => {
cy.get('.Select__control').click();
cy.get('input[type=text]').type('28 days{enter}');
cy.get('[data-test=time_compare]').find('.Select__control').click();
cy.get('[data-test=time_compare]')
.find('input[type=text]')
.type('28 days{enter}');

cy.get('.Select__control').click();
cy.get('input[type=text]').type('364 days{enter}');
cy.get('.Select__multi-value__label').contains('364 days');
});
cy.get('[data-test=time_compare]').find('.Select__control').click();
cy.get('[data-test=time_compare]')
.find('input[type=text]')
.type('364 days{enter}');
cy.get('[data-test=time_compare]')
.find('.Select__multi-value__label')
.contains('364 days');

cy.get('button.query').click();
cy.wait('@postJson');
Expand Down
12 changes: 3 additions & 9 deletions superset-frontend/cypress-base/cypress/integration/explore/control.test.ts
View file
Open in desktop
Expand Up @@ -46,16 +46,10 @@ describe('Datasource control', () => {
cy.get('.modal-footer button').contains('Save').click();
cy.get('.modal-footer button').contains('OK').click();
// select new metric
cy.get('.metrics-select:eq(0)').click();
cy.get('.metrics-select:eq(0) input[type="text"]')
cy.get('[data-test=metrics]')
.find('.Select__control input')
.focus()
.type(newMetricName);
cy.get('.metrics-select:eq(0) .Select__menu .Select__option')
.contains(newMetricName)
.click();
cy.get('.metrics-select:eq(0) .Select__multi-value__label')
.contains(newMetricName)
.click();
.type(newMetricName, { force: true });
// delete metric
cy.get('#datasource_menu').click();
cy.get('a').contains('Edit Datasource').click();
Expand Down
4 changes: 1 addition & 3 deletions superset-frontend/cypress-base/cypress/integration/explore/filter_box.test.js
View file
Open in desktop
Expand Up @@ -35,8 +35,6 @@ describe('Edit FilterBox Chart', () => {
it('should work with default date filter', () => {
verify(VIZ_DEFAULTS);
// Filter box should default to having a date filter with no filter selected
cy.get('div.filter_box').within(() => {
cy.get('span').contains('No filter');
});
cy.get('div.filter_box').contains('No filter');
});
});
4 changes: 2 additions & 2 deletions superset-frontend/cypress-base/cypress/integration/explore/visualizations/line.test.ts
View file
Open in desktop
Expand Up @@ -46,9 +46,9 @@ describe('Visualization > Line', () => {
});

it('should allow negative values in Y bounds', () => {
cy.get('#controlSections-tab-display').click();
cy.get('#controlSections-tab-display').click().wait(1000);
cy.get('span').contains('Y Axis Bounds').scrollIntoView();
cy.get('input[placeholder="Min"]').type('-0.1', { delay: 100 });
cy.get('input[placeholder="Min"]').type('-0.1', { delay: 100 }).wait(1000);
cy.get('.alert-warning').should('not.exist');
});

Expand Down
8 changes: 8 additions & 0 deletions superset-frontend/src/components/ListView/ListView.tsx
View file
Open in desktop
Expand Up @@ -50,6 +50,9 @@ const ListViewStyles = styled.div`
background: white;
position: sticky;
top: 0;
&:first-of-type {
padding-left: ${({ theme }) => theme.gridUnit * 4}px;
}
}
}
}
Expand Down Expand Up @@ -152,6 +155,11 @@ const ListViewStyles = styled.div`
overflow: hidden;
white-space: nowrap;
max-width: 300px;
line-height: 1;
vertical-align: middle;
&:first-of-type {
padding-left: ${({ theme }) => theme.gridUnit * 4}px;
}
}

.sort-icon {
Expand Down
8 changes: 7 additions & 1 deletion superset-frontend/src/explore/components/AdhocMetricEditPopover.jsx
View file
Open in desktop
Expand Up @@ -271,6 +271,7 @@ export default class AdhocMetricEditPopover extends React.Component {
className="adhoc-metric-edit-tab"
eventKey={EXPRESSION_TYPES.SQL}
title="Custom SQL"
data-test="adhoc-metric-edit-tab#custom"
>
{this.props.datasourceType !== 'druid' ? (
<FormGroup>
Expand Down Expand Up @@ -304,11 +305,16 @@ export default class AdhocMetricEditPopover extends React.Component {
bsStyle={hasUnsavedChanges && stateIsValid ? 'primary' : 'default'}
bsSize="small"
className="m-r-5"
data-test="AdhocMetricEdit#save"
onClick={this.onSave}
>
Save
</Button>
<Button bsSize="small" onClick={this.props.onClose}>
<Button
bsSize="small"
onClick={this.props.onClose}
data-test="AdhocMetricEdit#cancel"
>
Close
</Button>
<i
Expand Down
6 changes: 5 additions & 1 deletion superset-frontend/src/explore/components/AdhocMetricEditPopoverTitle.jsx
View file
Open in desktop
Expand Up @@ -70,6 +70,7 @@ export default class AdhocMetricEditPopoverTitle extends React.Component {
value={adhocMetric.hasCustomLabel ? adhocMetric.label : ''}
autoFocus
onChange={onChange}
data-test="AdhocMetricEditTitle#input"
/>
) : (
<OverlayTrigger
Expand All @@ -81,7 +82,10 @@ export default class AdhocMetricEditPopoverTitle extends React.Component {
onBlur={this.onBlur}
className="AdhocMetricEditPopoverTitle"
>
<span className="inline-editable">
<span
className="inline-editable"
data-test="AdhocMetricEditTitle#trigger"
>
{adhocMetric.hasCustomLabel ? adhocMetric.label : 'My Metric'}
&nbsp;
<i
Expand Down
69 changes: 44 additions & 25 deletions superset-frontend/src/explore/components/ControlPanelsContainer.jsx
View file
Open in desktop
Expand Up @@ -23,6 +23,7 @@ import { bindActionCreators } from 'redux';
import { connect } from 'react-redux';
import { Alert, Tab, Tabs } from 'react-bootstrap';
import { t } from '@superset-ui/translation';
import styled from '@superset-ui/style';

import ControlPanelSection from './ControlPanelSection';
import ControlRow from './ControlRow';
Expand All @@ -40,6 +41,25 @@ const propTypes = {
isDatasourceMetaLoading: PropTypes.bool.isRequired,
};

const Styles = styled.div`
max-height: 100%;
.remove-alert {
cursor: 'pointer';
}
#controlSections {
display: flex;
flex-direction: column;
max-height: 100%;
}
.nav-tabs {
flex: 0 0 1;
}
.tab-content {
overflow: auto;
flex: 1 1 100%;
}
`;

class ControlPanelsContainer extends React.Component {
constructor(props) {
super(props);
Expand Down Expand Up @@ -145,6 +165,7 @@ class ControlPanelsContainer extends React.Component {
</ControlPanelSection>
);
}

render() {
const querySectionsToRender = [];
const displaySectionsToRender = [];
Expand All @@ -170,32 +191,30 @@ class ControlPanelsContainer extends React.Component {
});

return (
<div className="scrollbar-container">
<div className="scrollbar-content">
{this.props.alert && (
<Alert bsStyle="warning">
{this.props.alert}
<i
role="button"
tabIndex={0}
className="fa fa-close pull-right"
onClick={this.removeAlert}
style={{ cursor: 'pointer' }}
/>
</Alert>
)}
<Tabs id="controlSections">
<Tab eventKey="query" title={t('Data')}>
{querySectionsToRender.map(this.renderControlPanelSection)}
<Styles>
{this.props.alert && (
<Alert bsStyle="warning">
{this.props.alert}
<i
role="button"
tabIndex={0}
className="fa fa-close pull-right"
onClick={this.removeAlert}
style={{ cursor: 'pointer' }}
/>
</Alert>
)}
<Tabs id="controlSections">
<Tab eventKey="query" title={t('Data')}>
{querySectionsToRender.map(this.renderControlPanelSection)}
</Tab>
{displaySectionsToRender.length > 0 && (
<Tab eventKey="display" title={t('Customize')}>
{displaySectionsToRender.map(this.renderControlPanelSection)}
</Tab>
{displaySectionsToRender.length > 0 && (
<Tab eventKey="display" title={t('Customize')}>
{displaySectionsToRender.map(this.renderControlPanelSection)}
</Tab>
)}
</Tabs>
</div>
</div>
)}
</Tabs>
</Styles>
);
}
}
Expand Down
26 changes: 19 additions & 7 deletions superset-frontend/src/explore/components/ExploreChartPanel.jsx
View file
Open in desktop
Expand Up @@ -18,8 +18,8 @@
*/
import React from 'react';
import PropTypes from 'prop-types';
import { Panel } from 'react-bootstrap';
import { ParentSize } from '@vx/responsive';
import styled from '@superset-ui/style';
import { chartPropShape } from '../../dashboard/util/propShapes';
import ChartContainer from '../../chart/ChartContainer';
import ExploreChartHeader from './ExploreChartHeader';
Expand Down Expand Up @@ -49,6 +49,19 @@ const propTypes = {
triggerRender: PropTypes.bool,
};

const Styles = styled.div`
background-color: ${({ theme }) => theme.colors.grayscale.light5};
padding: ${({ theme }) => theme.gridUnit * 4}px;
height: 100%;
display: flex;
flex-direction: column;
align-items: stretch;
align-content: stretch;
div:last-of-type {
flex-basis: 100%;
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is breaking FilterBox in Explore page. @rusackas

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dang! Thanks for the quick catch. I'll take a look.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Didn't get to fix it entirely last night... but it's not just that style... there are some dimensions that need to be added to a div... they're being added to other viz components, but not this one for some reason. Hope to have a PR soon.

`;

class ExploreChartPanel extends React.PureComponent {
renderChart() {
const { chart } = this.props;
Expand Down Expand Up @@ -113,13 +126,12 @@ class ExploreChartPanel extends React.PureComponent {
chart={this.props.chart}
/>
);

return (
<div className="chart-container">
<Panel style={{ height: this.props.height }}>
<Panel.Heading>{header}</Panel.Heading>
<Panel.Body>{this.renderChart()}</Panel.Body>
</Panel>
</div>
<Styles className="chart-container">
<div>{header}</div>
<div>{this.renderChart()}</div>
</Styles>
Copy link
Member

@ktmud ktmud Aug 3, 2020 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rusackas by not using the Panel component, you also missed some CSS styles for .panel-heading and .panel-body.

After

image

Before

image

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just pushed a fix. I'm using the raw boostrap classes for now. Let's refactor it into Antd components or a more self-contained styled component in the future.

);
}
}
Expand Down