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

fix(explore): Small changes in metadata bar copy and tooltip placement #21952

Merged
merged 3 commits into from
Oct 28, 2022
Merged

fix(explore): Small changes in metadata bar copy and tooltip placement #21952

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
82be467
fix(explore): Small change in metadata bar copy and tooltip placement
kgabryje Oct 27, 2022
d91dd9e
Fix tests
kgabryje Oct 27, 2022
71c6322
Add tooltipPlacement prop to MetadataBar instead of hardcoding "bottom"
kgabryje Oct 27, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion superset-frontend/cypress-base/cypress/integration/explore/chart.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,9 @@ function verifyMetabar(text) {

function saveAndVerifyDashboard(number) {
saveChartToDashboard(`${number} - Sample dashboard`);
verifyMetabar(`Added to ${number} dashboard(s)`);
verifyMetabar(
number > 1 ? `Added to ${number} dashboards` : 'Added to 1 dashboard',
);
openDashboardsAddedTo();
verifyDashboardsSubmenuItem(`${number} - Sample dashboard`);
}
Expand Down
5 changes: 4 additions & 1 deletion superset-frontend/src/components/MetadataBar/MetadataBar.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,10 @@ const Item = ({
</StyledItem>
);
return isTruncated || collapsed || (tooltip && tooltip !== title) ? (
<Tooltip title={<TootipContent>{tooltip}</TootipContent>}>
<Tooltip
placement="bottom"
michael-s-molina marked this conversation as resolved.
Show resolved Hide resolved
title={<TootipContent>{tooltip}</TootipContent>}
>
{content}
</Tooltip>
) : (
Expand Down
26 changes: 20 additions & 6 deletions superset-frontend/src/explore/components/ExploreChartHeader/ExploreChartHeader.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -159,21 +159,35 @@ test('Cancelling changes to the properties should reset previous properties', as
test('renders the metadata bar when saved', async () => {
const props = createProps({ showTitlePanelItems: true });
render(<ExploreHeader {...props} />, { useRedux: true });
expect(
await screen.findByText('Added to 1 dashboard(s)'),
).toBeInTheDocument();
expect(await screen.findByText('Added to 1 dashboard')).toBeInTheDocument();
expect(await screen.findByText('Simple description')).toBeInTheDocument();
expect(await screen.findByText('John Doe')).toBeInTheDocument();
expect(await screen.findByText('2 days ago')).toBeInTheDocument();
});

test('Changes "Added to X dashboards" to plural when more than 1 dashboard', async () => {
const props = createProps({ showTitlePanelItems: true });
render(
<ExploreHeader
{...props}
metadata={{
...props.metadata,
dashboards: [
{ id: 1, dashboard_title: 'Test' },
{ id: 2, dashboard_title: 'Test2' },
],
}}
/>,
{ useRedux: true },
);
expect(await screen.findByText('Added to 2 dashboards')).toBeInTheDocument();
});

test('does not render the metadata bar when not saved', async () => {
const props = createProps({ showTitlePanelItems: true, slice: null });
render(<ExploreHeader {...props} />, { useRedux: true });
await waitFor(() =>
expect(
screen.queryByText('Added to 1 dashboard(s)'),
).not.toBeInTheDocument(),
expect(screen.queryByText('Added to 1 dashboard')).not.toBeInTheDocument(),
);
});

Expand Down
10 changes: 8 additions & 2 deletions superset-frontend/src/explore/components/ExploreChartHeader/index.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import {
logging,
SupersetClient,
t,
tn,
} from '@superset-ui/core';
import { toggleActive, deleteActiveReport } from 'src/reports/actions/reports';
import { chartPropShape } from 'src/dashboard/util/propShapes';
Expand Down Expand Up @@ -161,12 +162,17 @@ export const ExploreChartHeader = ({
type: MetadataType.DASHBOARDS,
title:
metadata.dashboards.length > 0
? t('Added to %s dashboard(s)', metadata.dashboards.length)
? tn(
'Added to 1 dashboard',
'Added to %s dashboards',
metadata.dashboards.length,
metadata.dashboards.length,
)
: t('Not added to any dashboard'),
description:
metadata.dashboards.length > 0
? t(
'You can preview the list of dashboards on the chart settings dropdown.',
'You can preview the list of dashboards in the chart settings dropdown.',
)
: undefined,
});
Expand Down