You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by vi...@apache.org on 2022/04/28 07:28:56 UTC
[superset] branch master updated: chore: fix explore pills (#19866)
This is an automated email from the ASF dual-hosted git repository.
villebro pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/superset.git
The following commit(s) were added to refs/heads/master by this push:
new 3d2fec9604 chore: fix explore pills (#19866)
3d2fec9604 is described below
commit 3d2fec96048bbe0a48d2ca88cac6a9e8fc0f92a4
Author: Ville Brofeldt <33...@users.noreply.github.com>
AuthorDate: Thu Apr 28 10:28:47 2022 +0300
chore: fix explore pills (#19866)
* chore: fix explore pills
* fix tests
* address comments
* add test and remove redundant div
* switch to dark text
---
.../cypress/integration/explore/control.test.ts | 4 +-
.../src/components/AlteredSliceTag/index.jsx | 18 ++++--
.../src/components/CachedLabel/index.tsx | 2 +-
.../src/components/Label/Label.stories.tsx | 3 +-
superset-frontend/src/components/Label/index.tsx | 22 ++++++--
.../components/DataTableControl/RowCount.test.tsx | 9 ++-
.../explore/components/DataTableControl/index.tsx | 8 +--
.../DataTablesPane/DataTablesPane.test.tsx | 8 +--
.../RowCountLabel/RowCountLabel.stories.tsx | 13 ++---
.../RowCountLabel/RowCountLabel.test.jsx | 50 -----------------
.../RowCountLabel/RowCountLabel.test.tsx | 65 ++++++++++++++++++++++
.../src/explore/components/RowCountLabel/index.tsx | 28 +++++-----
12 files changed, 133 insertions(+), 97 deletions(-)
diff --git a/superset-frontend/cypress-base/cypress/integration/explore/control.test.ts b/superset-frontend/cypress-base/cypress/integration/explore/control.test.ts
index 97dfd2945a..e2aec6b7b9 100644
--- a/superset-frontend/cypress-base/cypress/integration/explore/control.test.ts
+++ b/superset-frontend/cypress-base/cypress/integration/explore/control.test.ts
@@ -122,12 +122,12 @@ describe('Test datatable', () => {
});
it('Data Pane opens and loads results', () => {
cy.contains('Results').click();
- cy.get('[data-test="row-count-label"]').contains('26 rows retrieved');
+ cy.get('[data-test="row-count-label"]').contains('26 rows');
cy.get('.ant-empty-description').should('not.exist');
});
it('Datapane loads view samples', () => {
cy.contains('Samples').click();
- cy.get('[data-test="row-count-label"]').contains('1k rows retrieved');
+ cy.get('[data-test="row-count-label"]').contains('1k rows');
cy.get('.ant-empty-description').should('not.exist');
});
});
diff --git a/superset-frontend/src/components/AlteredSliceTag/index.jsx b/superset-frontend/src/components/AlteredSliceTag/index.jsx
index e4a0d1bdeb..2faa62c890 100644
--- a/superset-frontend/src/components/AlteredSliceTag/index.jsx
+++ b/superset-frontend/src/components/AlteredSliceTag/index.jsx
@@ -19,7 +19,7 @@
import React from 'react';
import PropTypes from 'prop-types';
import { isEqual, isEmpty } from 'lodash';
-import { t } from '@superset-ui/core';
+import { styled, t } from '@superset-ui/core';
import { sanitizeFormData } from 'src/explore/exploreUtils/formData';
import getControlsForVizType from 'src/utils/getControlsForVizType';
import { safeStringify } from 'src/utils/safeStringify';
@@ -32,6 +32,18 @@ const propTypes = {
currentFormData: PropTypes.object.isRequired,
};
+const StyledLabel = styled.span`
+ ${({ theme }) => `
+ font-size: ${theme.typography.sizes.s}px;
+ color: ${theme.colors.grayscale.dark1};
+ background-color: ${theme.colors.alert.base};
+
+ &: hover {
+ background-color: ${theme.colors.alert.dark1};
+ }
+ `}
+`;
+
function alterForComparison(value) {
// Considering `[]`, `{}`, `null` and `undefined` as identical
// for this purpose
@@ -183,9 +195,7 @@ export default class AlteredSliceTag extends React.Component {
renderTriggerNode() {
return (
<Tooltip id="difference-tooltip" title={t('Click to see difference')}>
- <span className="label label-warning" style={{ fontSize: '12px' }}>
- {t('Altered')}
- </span>
+ <StyledLabel className="label">{t('Altered')}</StyledLabel>
</Tooltip>
);
}
diff --git a/superset-frontend/src/components/CachedLabel/index.tsx b/superset-frontend/src/components/CachedLabel/index.tsx
index a401ab1e0e..a6c74cb8eb 100644
--- a/superset-frontend/src/components/CachedLabel/index.tsx
+++ b/superset-frontend/src/components/CachedLabel/index.tsx
@@ -48,7 +48,7 @@ const CacheLabel: React.FC<CacheLabelProps> = ({
onMouseOver={() => setHovered(true)}
onMouseOut={() => setHovered(false)}
>
- {t('cached')} <i className="fa fa-refresh" />
+ {t('Cached')} <i className="fa fa-refresh" />
</Label>
</Tooltip>
);
diff --git a/superset-frontend/src/components/Label/Label.stories.tsx b/superset-frontend/src/components/Label/Label.stories.tsx
index 07fc7c09fa..9363cc80d3 100644
--- a/superset-frontend/src/components/Label/Label.stories.tsx
+++ b/superset-frontend/src/components/Label/Label.stories.tsx
@@ -26,8 +26,9 @@ export default {
excludeStories: 'options',
};
-export const options = [
+export const options: Type[] = [
'default',
+ 'alert',
'info',
'success',
'warning',
diff --git a/superset-frontend/src/components/Label/index.tsx b/superset-frontend/src/components/Label/index.tsx
index e66437da7d..eb2aff41d0 100644
--- a/superset-frontend/src/components/Label/index.tsx
+++ b/superset-frontend/src/components/Label/index.tsx
@@ -23,6 +23,7 @@ import { useTheme } from '@superset-ui/core';
export type OnClickHandler = React.MouseEventHandler<HTMLElement>;
export type Type =
+ | 'alert'
| 'success'
| 'warning'
| 'danger'
@@ -44,9 +45,17 @@ export interface LabelProps extends React.HTMLAttributes<HTMLSpanElement> {
export default function Label(props: LabelProps) {
const theme = useTheme();
const { colors, transitionTiming } = theme;
- const { type, onClick, children, ...rest } = props;
- const { primary, secondary, grayscale, success, warning, error, info } =
- colors;
+ const { type = 'default', onClick, children, ...rest } = props;
+ const {
+ alert,
+ primary,
+ secondary,
+ grayscale,
+ success,
+ warning,
+ error,
+ info,
+ } = colors;
let backgroundColor = grayscale.light3;
let backgroundColorHover = onClick ? primary.light2 : grayscale.light3;
@@ -54,11 +63,14 @@ export default function Label(props: LabelProps) {
let borderColorHover = onClick ? primary.light1 : 'transparent';
let color = grayscale.dark1;
- if (type && type !== 'default') {
+ if (type !== 'default') {
color = grayscale.light4;
let baseColor;
- if (type === 'success') {
+ if (type === 'alert') {
+ color = grayscale.dark1;
+ baseColor = alert;
+ } else if (type === 'success') {
baseColor = success;
} else if (type === 'warning') {
baseColor = warning;
diff --git a/superset-frontend/src/explore/components/DataTableControl/RowCount.test.tsx b/superset-frontend/src/explore/components/DataTableControl/RowCount.test.tsx
index be3f0e980c..6ebbcef77b 100644
--- a/superset-frontend/src/explore/components/DataTableControl/RowCount.test.tsx
+++ b/superset-frontend/src/explore/components/DataTableControl/RowCount.test.tsx
@@ -20,9 +20,14 @@ import React from 'react';
import { render, screen } from 'spec/helpers/testing-library';
import { RowCount } from '.';
-test('Render a RowCount', () => {
+test('Render a RowCount with a single row', () => {
+ render(<RowCount data={[{}]} loading={false} />);
+ expect(screen.getByText('1 row')).toBeInTheDocument();
+});
+
+test('Render a RowCount with multiple rows', () => {
render(<RowCount data={[{}, {}, {}]} loading={false} />);
- expect(screen.getByText('3 rows retrieved')).toBeInTheDocument();
+ expect(screen.getByText('3 rows')).toBeInTheDocument();
});
test('Render a RowCount on loading', () => {
diff --git a/superset-frontend/src/explore/components/DataTableControl/index.tsx b/superset-frontend/src/explore/components/DataTableControl/index.tsx
index 7a25c374bd..791f5f8ff3 100644
--- a/superset-frontend/src/explore/components/DataTableControl/index.tsx
+++ b/superset-frontend/src/explore/components/DataTableControl/index.tsx
@@ -127,13 +127,7 @@ export const RowCount = ({
}: {
data?: Record<string, any>[];
loading: boolean;
-}) => (
- <RowCountLabel
- rowcount={data?.length ?? 0}
- loading={loading}
- suffix={t('rows retrieved')}
- />
-);
+}) => <RowCountLabel rowcount={data?.length ?? 0} loading={loading} />;
enum FormatPickerValue {
Formatted,
diff --git a/superset-frontend/src/explore/components/DataTablesPane/DataTablesPane.test.tsx b/superset-frontend/src/explore/components/DataTablesPane/DataTablesPane.test.tsx
index 786150449e..04869f5f10 100644
--- a/superset-frontend/src/explore/components/DataTablesPane/DataTablesPane.test.tsx
+++ b/superset-frontend/src/explore/components/DataTablesPane/DataTablesPane.test.tsx
@@ -103,7 +103,7 @@ describe('DataTablesPane', () => {
useRedux: true,
});
userEvent.click(screen.getByText('Results'));
- expect(await screen.findByText('0 rows retrieved')).toBeVisible();
+ expect(await screen.findByText('0 rows')).toBeVisible();
expect(await screen.findByLabelText('Collapse data panel')).toBeVisible();
localStorage.clear();
});
@@ -114,7 +114,7 @@ describe('DataTablesPane', () => {
useRedux: true,
});
userEvent.click(screen.getByText('Samples'));
- expect(await screen.findByText('0 rows retrieved')).toBeVisible();
+ expect(await screen.findByText('0 rows')).toBeVisible();
expect(await screen.findByLabelText('Collapse data panel')).toBeVisible();
});
@@ -158,7 +158,7 @@ describe('DataTablesPane', () => {
},
);
userEvent.click(screen.getByText('Results'));
- expect(await screen.findByText('1 rows retrieved')).toBeVisible();
+ expect(await screen.findByText('1 row')).toBeVisible();
userEvent.click(screen.getByLabelText('Copy'));
expect(copyToClipboardSpy).toHaveBeenCalledWith(
@@ -210,7 +210,7 @@ describe('DataTablesPane', () => {
},
);
userEvent.click(screen.getByText('Results'));
- expect(await screen.findByText('2 rows retrieved')).toBeVisible();
+ expect(await screen.findByText('2 rows')).toBeVisible();
expect(screen.getByText('Action')).toBeVisible();
expect(screen.getByText('Horror')).toBeVisible();
diff --git a/superset-frontend/src/explore/components/RowCountLabel/RowCountLabel.stories.tsx b/superset-frontend/src/explore/components/RowCountLabel/RowCountLabel.stories.tsx
index 716830f9ca..558f14f308 100644
--- a/superset-frontend/src/explore/components/RowCountLabel/RowCountLabel.stories.tsx
+++ b/superset-frontend/src/explore/components/RowCountLabel/RowCountLabel.stories.tsx
@@ -17,17 +17,21 @@
* under the License.
*/
import React from 'react';
-import RowCountLabel from '.';
+import RowCountLabel, { RowCountLabelProps } from '.';
export default {
title: 'RowCountLabel',
component: RowCountLabel,
};
-const options = {
+const options: { [key in string]: RowCountLabelProps } = {
loading: {
loading: true,
},
+ single: {
+ rowcount: 1,
+ limit: 100,
+ },
full: {
rowcount: 100,
limit: 100,
@@ -36,10 +40,6 @@ const options = {
rowcount: 50,
limit: 100,
},
- suffix: {
- rowcount: 1,
- suffix: 'suffix',
- },
};
export const RowCountLabelGallery = () => (
@@ -51,7 +51,6 @@ export const RowCountLabelGallery = () => (
loading={options[name].loading}
rowcount={options[name].rowcount}
limit={options[name].limit}
- suffix={options[name].suffix}
/>
</>
))}
diff --git a/superset-frontend/src/explore/components/RowCountLabel/RowCountLabel.test.jsx b/superset-frontend/src/explore/components/RowCountLabel/RowCountLabel.test.jsx
deleted file mode 100644
index 452f68a745..0000000000
--- a/superset-frontend/src/explore/components/RowCountLabel/RowCountLabel.test.jsx
+++ /dev/null
@@ -1,50 +0,0 @@
-/**
- * Licensed to the Apache Software Foundation (ASF) under one
- * or more contributor license agreements. See the NOTICE file
- * distributed with this work for additional information
- * regarding copyright ownership. The ASF licenses this file
- * to you under the Apache License, Version 2.0 (the
- * "License"); you may not use this file except in compliance
- * with the License. You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing,
- * software distributed under the License is distributed on an
- * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
- * KIND, either express or implied. See the License for the
- * specific language governing permissions and limitations
- * under the License.
- */
-import React from 'react';
-import { shallow } from 'enzyme';
-
-import Label from 'src/components/Label';
-import { Tooltip } from 'src/components/Tooltip';
-import RowCountLabel from '.';
-
-describe('RowCountLabel', () => {
- const defaultProps = {
- rowcount: 51,
- limit: 100,
- };
-
- it('is valid', () => {
- expect(React.isValidElement(<RowCountLabel {...defaultProps} />)).toBe(
- true,
- );
- });
- it('renders a Label and a Tooltip', () => {
- const wrapper = shallow(<RowCountLabel {...defaultProps} />);
- expect(wrapper.find(Label)).toExist();
- expect(wrapper.find(Tooltip)).toExist();
- });
- it('renders a danger when limit is reached', () => {
- const props = {
- rowcount: 100,
- limit: 100,
- };
- const wrapper = shallow(<RowCountLabel {...props} />);
- expect(wrapper.find(Label).first().props().type).toBe('danger');
- });
-});
diff --git a/superset-frontend/src/explore/components/RowCountLabel/RowCountLabel.test.tsx b/superset-frontend/src/explore/components/RowCountLabel/RowCountLabel.test.tsx
new file mode 100644
index 0000000000..2e7a44c15e
--- /dev/null
+++ b/superset-frontend/src/explore/components/RowCountLabel/RowCountLabel.test.tsx
@@ -0,0 +1,65 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import React from 'react';
+import { render, screen } from 'spec/helpers/testing-library';
+import userEvent from '@testing-library/user-event';
+
+import RowCountLabel from '.';
+
+test('RowCountLabel renders singular result', () => {
+ render(<RowCountLabel rowcount={1} limit={100} />);
+ const expectedText = '1 row';
+ expect(screen.getByText(expectedText)).toBeInTheDocument();
+ userEvent.hover(screen.getByText(expectedText));
+ expect(screen.queryByRole('tooltip')).not.toBeInTheDocument();
+});
+
+test('RowCountLabel renders plural result', () => {
+ render(<RowCountLabel rowcount={2} limit={100} />);
+ const expectedText = '2 rows';
+ expect(screen.getByText(expectedText)).toBeInTheDocument();
+ userEvent.hover(screen.getByText(expectedText));
+ expect(screen.queryByRole('tooltip')).not.toBeInTheDocument();
+});
+
+test('RowCountLabel renders formatted result', () => {
+ render(<RowCountLabel rowcount={1000} limit={10000} />);
+ const expectedText = '1k rows';
+ expect(screen.getByText(expectedText)).toBeInTheDocument();
+ userEvent.hover(screen.getByText(expectedText));
+ expect(screen.queryByRole('tooltip')).not.toBeInTheDocument();
+});
+
+test('RowCountLabel renders limit with danger and tooltip', async () => {
+ render(<RowCountLabel rowcount={100} limit={100} />);
+ const expectedText = '100 rows';
+ expect(screen.getByText(expectedText)).toBeInTheDocument();
+ userEvent.hover(screen.getByText(expectedText));
+ const tooltip = await screen.findByRole('tooltip');
+ expect(tooltip).toHaveTextContent('Limit reached');
+ expect(tooltip).toHaveStyle('background: rgba(0, 0, 0, 0.902);');
+});
+
+test('RowCountLabel renders loading', () => {
+ render(<RowCountLabel loading />);
+ const expectedText = 'Loading...';
+ expect(screen.getByText(expectedText)).toBeInTheDocument();
+ userEvent.hover(screen.getByText(expectedText));
+ expect(screen.queryByRole('tooltip')).not.toBeInTheDocument();
+});
diff --git a/superset-frontend/src/explore/components/RowCountLabel/index.tsx b/superset-frontend/src/explore/components/RowCountLabel/index.tsx
index c612856817..3597e49724 100644
--- a/superset-frontend/src/explore/components/RowCountLabel/index.tsx
+++ b/superset-frontend/src/explore/components/RowCountLabel/index.tsx
@@ -17,36 +17,36 @@
* under the License.
*/
import React from 'react';
-import { getNumberFormatter, t } from '@superset-ui/core';
+import { getNumberFormatter, t, tn } from '@superset-ui/core';
import Label from 'src/components/Label';
import { Tooltip } from 'src/components/Tooltip';
type RowCountLabelProps = {
- rowcount: number;
+ rowcount?: number;
limit?: number;
- suffix?: string;
loading?: boolean;
};
export default function RowCountLabel(props: RowCountLabelProps) {
- const { rowcount = 0, limit, suffix = t('rows'), loading } = props;
+ const { rowcount = 0, limit, loading } = props;
const limitReached = rowcount === limit;
const type =
limitReached || (rowcount === 0 && !loading) ? 'danger' : 'default';
const formattedRowCount = getNumberFormatter()(rowcount);
- const tooltip = (
- <span>
- {limitReached && <div>{t('Limit reached')}</div>}
- {loading ? 'Loading' : rowcount}
- </span>
+ const label = (
+ <Label type={type} data-test="row-count-label">
+ {loading
+ ? t('Loading...')
+ : tn('%s row', '%s rows', rowcount, formattedRowCount)}
+ </Label>
);
- return (
- <Tooltip id="tt-rowcount-tooltip" title={tooltip}>
- <Label type={type} data-test="row-count-label">
- {loading ? 'Loading...' : `${formattedRowCount} ${suffix}`}
- </Label>
+ return limitReached ? (
+ <Tooltip id="tt-rowcount-tooltip" title={<span>{t('Limit reached')}</span>}>
+ {label}
</Tooltip>
+ ) : (
+ label
);
}