Skip to content
Snippets Groups Projects
Unverified Commit 011335a8 authored by Ryan Laurie's avatar Ryan Laurie Committed by GitHub
Browse files

Sidesheet Polish :flag_pl: (#47935)

* size and padding fixes

* linkify and align stuff in question details

* reorganize and divide question overflow menu

* hide history for analytics content

* fix verification icon alignment

* whoopsie lets commit all the files

* fix css file name
parent af906dec
No related branches found
No related tags found
No related merge requests found
Showing
with 224 additions and 55 deletions
.IconMargin {
margin-top: 0.25rem;
}
...@@ -5,7 +5,7 @@ import { skipToken, useGetUserQuery } from "metabase/api"; ...@@ -5,7 +5,7 @@ import { skipToken, useGetUserQuery } from "metabase/api";
import { alpha, color } from "metabase/lib/colors"; import { alpha, color } from "metabase/lib/colors";
import { useSelector } from "metabase/lib/redux"; import { useSelector } from "metabase/lib/redux";
import { getRelativeTime } from "metabase/lib/time"; import { getRelativeTime } from "metabase/lib/time";
import { Flex, Icon, Text as UIText } from "metabase/ui"; import { FixedSizeIcon, Flex, Icon, Text as UIText } from "metabase/ui";
import { import {
getIconForReview, getIconForReview,
getLatestModerationReview, getLatestModerationReview,
...@@ -14,13 +14,13 @@ import { ...@@ -14,13 +14,13 @@ import {
import type Question from "metabase-lib/v1/Question"; import type Question from "metabase-lib/v1/Question";
import type { ModerationReview } from "metabase-types/api"; import type { ModerationReview } from "metabase-types/api";
import Styles from "./ModerationReview.module.css";
import { import {
Container, Container,
Text, Text,
TextContainer, TextContainer,
Time, Time,
} from "./ModerationReviewBanner.styled"; } from "./ModerationReviewBanner.styled";
const ICON_BUTTON_SIZE = 16; const ICON_BUTTON_SIZE = 16;
interface ModerationReviewBannerProps { interface ModerationReviewBannerProps {
...@@ -93,8 +93,13 @@ export const ModerationReviewText = ({ question }: { question: Question }) => { ...@@ -93,8 +93,13 @@ export const ModerationReviewText = ({ question }: { question: Question }) => {
); );
return ( return (
<Flex gap="sm" align="center"> <Flex gap="sm" align="top">
<Icon name={iconName} color={color(iconColor)} size={ICON_BUTTON_SIZE} /> <FixedSizeIcon
name={iconName}
color={color(iconColor)}
size={ICON_BUTTON_SIZE}
className={Styles.IconMargin}
/>
<UIText> <UIText>
{bannerText} {relativeCreationTime} {bannerText} {relativeCreationTime}
</UIText> </UIText>
......
...@@ -35,7 +35,7 @@ const getSchemaName = props => { ...@@ -35,7 +35,7 @@ const getSchemaName = props => {
const getReloadInterval = (_state, _props, tables = []) => const getReloadInterval = (_state, _props, tables = []) =>
tables.some(t => isSyncInProgress(t)) ? RELOAD_INTERVAL : 0; tables.some(t => isSyncInProgress(t)) ? RELOAD_INTERVAL : 0;
const getTableUrl = (table, metadata) => { export const getTableUrl = (table, metadata) => {
const metadataTable = metadata?.table(table.id); const metadataTable = metadata?.table(table.id);
return ML_Urls.getUrl(metadataTable?.newQuestion(), { clean: false }); return ML_Urls.getUrl(metadataTable?.newQuestion(), { clean: false });
}; };
......
...@@ -10,7 +10,7 @@ import Styles from "./sidesheet.module.css"; ...@@ -10,7 +10,7 @@ import Styles from "./sidesheet.module.css";
export const SidesheetTabPanelContainer = ( export const SidesheetTabPanelContainer = (
props: MantineStyleSystemProps & { children: React.ReactNode }, props: MantineStyleSystemProps & { children: React.ReactNode },
) => ( ) => (
<Box className={Styles.OverflowAuto} p="lg" {...props}> <Box className={Styles.OverflowAuto} p="xl" {...props}>
<div>{props.children}</div> <div>{props.children}</div>
</Box> </Box>
); );
...@@ -5,7 +5,7 @@ import { Component, createRef } from "react"; ...@@ -5,7 +5,7 @@ import { Component, createRef } from "react";
import EntityMenuItem from "metabase/components/EntityMenuItem"; import EntityMenuItem from "metabase/components/EntityMenuItem";
import EntityMenuTrigger from "metabase/components/EntityMenuTrigger"; import EntityMenuTrigger from "metabase/components/EntityMenuTrigger";
import CS from "metabase/css/core/index.css"; import CS from "metabase/css/core/index.css";
import { Popover } from "metabase/ui"; import { Divider, Popover } from "metabase/ui";
/** /**
* @deprecated: use Menu from "metabase/ui" * @deprecated: use Menu from "metabase/ui"
...@@ -98,6 +98,14 @@ class EntityMenu extends Component { ...@@ -98,6 +98,14 @@ class EntityMenu extends Component {
const key = item.key ?? item.title; const key = item.key ?? item.title;
if (item.separator) {
return (
<li key={key}>
<Divider m="sm" />
</li>
);
}
if (item.content) { if (item.content) {
return ( return (
<li key={key} data-testid={item.testId}> <li key={key} data-testid={item.testId}>
......
import type { FocusEvent, SetStateAction } from "react"; import type { FocusEvent, SetStateAction } from "react";
import { useCallback, useState } from "react"; import { useCallback, useMemo, useState } from "react";
import { useMount } from "react-use"; import { useMount } from "react-use";
import { t } from "ttag"; import { t } from "ttag";
import ErrorBoundary from "metabase/ErrorBoundary"; import ErrorBoundary from "metabase/ErrorBoundary";
import { isInstanceAnalyticsCollection } from "metabase/collections/utils";
import { import {
Sidesheet, Sidesheet,
SidesheetCard, SidesheetCard,
...@@ -70,6 +71,13 @@ export function DashboardInfoSidebar({ ...@@ -70,6 +71,13 @@ export function DashboardInfoSidebar({
query: { model_type: "dashboard", model_id: dashboard.id }, query: { model_type: "dashboard", model_id: dashboard.id },
}); });
const isIADashboard = useMemo(
() =>
dashboard.collection &&
isInstanceAnalyticsCollection(dashboard?.collection),
[dashboard.collection],
);
const currentUser = useSelector(getUser); const currentUser = useSelector(getUser);
const dispatch = useDispatch(); const dispatch = useDispatch();
...@@ -123,9 +131,11 @@ export function DashboardInfoSidebar({ ...@@ -123,9 +131,11 @@ export function DashboardInfoSidebar({
defaultValue={Tab.Overview} defaultValue={Tab.Overview}
className={SidesheetS.FlexScrollContainer} className={SidesheetS.FlexScrollContainer}
> >
<Tabs.List mx="lg"> <Tabs.List mx="xl">
<Tabs.Tab value={Tab.Overview}>{t`Overview`}</Tabs.Tab> <Tabs.Tab value={Tab.Overview}>{t`Overview`}</Tabs.Tab>
<Tabs.Tab value={Tab.History}>{t`History`}</Tabs.Tab> {!isIADashboard && (
<Tabs.Tab value={Tab.History}>{t`History`}</Tabs.Tab>
)}
</Tabs.List> </Tabs.List>
<SidesheetTabPanelContainer> <SidesheetTabPanelContainer>
<Tabs.Panel value={Tab.Overview}> <Tabs.Panel value={Tab.Overview}>
......
import userEvent from "@testing-library/user-event"; import userEvent from "@testing-library/user-event";
import { screen } from "__support__/ui"; import { screen } from "__support__/ui";
import {
createMockCollection,
createMockDashboard,
} from "metabase-types/api/mocks";
import { setupEnterprise } from "./setup"; import { setupEnterprise } from "./setup";
const tokenFeatures = { const tokenFeatures = {
cache_granular_controls: true, cache_granular_controls: true,
audit_app: true,
}; };
describe("DashboardInfoSidebar > premium enterprise", () => { describe("DashboardInfoSidebar > premium enterprise", () => {
...@@ -30,4 +35,17 @@ describe("DashboardInfoSidebar > premium enterprise", () => { ...@@ -30,4 +35,17 @@ describe("DashboardInfoSidebar > premium enterprise", () => {
expect(await screen.findByText("Caching settings")).toBeInTheDocument(); expect(await screen.findByText("Caching settings")).toBeInTheDocument();
}); });
it("should hide history for instance analytics dashboard", async () => {
await setupEnterprise(
{
dashboard: createMockDashboard({
collection: createMockCollection({ type: "instance-analytics" }),
}),
},
tokenFeatures,
);
expect(screen.queryByText("History")).not.toBeInTheDocument();
});
}); });
...@@ -125,6 +125,15 @@ export const QuestionActions = ({ ...@@ -125,6 +125,15 @@ export const QuestionActions = ({
const extraButtons = []; const extraButtons = [];
if (isQuestion || isMetric) {
extraButtons.push({
title: t`Add to dashboard`,
icon: "add_to_dash",
action: () => onOpenModal(MODAL_TYPES.ADD_TO_DASHBOARD),
testId: ADD_TO_DASH_TESTID,
});
}
if ( if (
isMetabotEnabled && isMetabotEnabled &&
isModel && isModel &&
...@@ -168,15 +177,26 @@ export const QuestionActions = ({ ...@@ -168,15 +177,26 @@ export const QuestionActions = ({
} }
} }
if (isQuestion || isMetric) { if (hasCollectionPermissions) {
extraButtons.push({ if (isQuestion) {
title: t`Add to dashboard`, extraButtons.push({
icon: "add_to_dash", title: t`Turn into a model`,
action: () => onOpenModal(MODAL_TYPES.ADD_TO_DASHBOARD), icon: "model",
testId: ADD_TO_DASH_TESTID, action: handleTurnToModel,
}); testId: TURN_INTO_DATASET_TESTID,
});
}
if (isModel) {
extraButtons.push({
title: t`Turn back to saved question`,
icon: "insight",
action: onTurnModelIntoQuestion,
});
}
} }
extraButtons.push(...PLUGIN_QUERY_BUILDER_HEADER.extraButtons(question));
if (enableSettingsSidebar) { if (enableSettingsSidebar) {
extraButtons.push({ extraButtons.push({
title: t`Edit settings`, title: t`Edit settings`,
...@@ -187,6 +207,10 @@ export const QuestionActions = ({ ...@@ -187,6 +207,10 @@ export const QuestionActions = ({
} }
if (hasCollectionPermissions) { if (hasCollectionPermissions) {
extraButtons.push({
separator: true,
key: "move-separator",
});
extraButtons.push({ extraButtons.push({
title: t`Move`, title: t`Move`,
icon: "move", icon: "move",
...@@ -205,26 +229,10 @@ export const QuestionActions = ({ ...@@ -205,26 +229,10 @@ export const QuestionActions = ({
} }
if (hasCollectionPermissions) { if (hasCollectionPermissions) {
if (isQuestion) { extraButtons.push({
extraButtons.push({ separator: true,
title: t`Turn into a model`, key: "trash-separator",
icon: "model", });
action: handleTurnToModel,
testId: TURN_INTO_DATASET_TESTID,
});
}
if (isModel) {
extraButtons.push({
title: t`Turn back to saved question`,
icon: "insight",
action: onTurnModelIntoQuestion,
});
}
}
extraButtons.push(...PLUGIN_QUERY_BUILDER_HEADER.extraButtons(question));
if (hasCollectionPermissions) {
extraButtons.push({ extraButtons.push({
title: t`Move to trash`, title: t`Move to trash`,
icon: "trash", icon: "trash",
......
...@@ -2,13 +2,19 @@ import cx from "classnames"; ...@@ -2,13 +2,19 @@ import cx from "classnames";
import { useState } from "react"; import { useState } from "react";
import { c, t } from "ttag"; import { c, t } from "ttag";
import { getTableUrl } from "metabase/browse/containers/TableBrowser/TableBrowser";
import { SidesheetCardSection } from "metabase/common/components/Sidesheet"; import { SidesheetCardSection } from "metabase/common/components/Sidesheet";
import DateTime from "metabase/components/DateTime"; import DateTime from "metabase/components/DateTime";
import Link from "metabase/core/components/Link";
import Styles from "metabase/css/core/index.css"; import Styles from "metabase/css/core/index.css";
import { useSelector } from "metabase/lib/redux";
import * as Urls from "metabase/lib/urls";
import { getUserName } from "metabase/lib/user"; import { getUserName } from "metabase/lib/user";
import { getMetadata } from "metabase/selectors/metadata";
import { QuestionPublicLinkPopover } from "metabase/sharing/components/PublicLinkPopover"; import { QuestionPublicLinkPopover } from "metabase/sharing/components/PublicLinkPopover";
import { Box, Flex, Icon, Text } from "metabase/ui"; import { Box, Flex, FixedSizeIcon as Icon, Text } from "metabase/ui";
import type Question from "metabase-lib/v1/Question"; import type Question from "metabase-lib/v1/Question";
import type { Database } from "metabase-types/api";
import SidebarStyles from "./QuestionInfoSidebar.module.css"; import SidebarStyles from "./QuestionInfoSidebar.module.css";
...@@ -21,8 +27,8 @@ export const QuestionDetails = ({ question }: { question: Question }) => { ...@@ -21,8 +27,8 @@ export const QuestionDetails = ({ question }: { question: Question }) => {
<> <>
<SidesheetCardSection title={t`Creator and last editor`}> <SidesheetCardSection title={t`Creator and last editor`}>
{lastEditInfo && ( {lastEditInfo && (
<Flex gap="sm" align="center"> <Flex gap="sm" align="top">
<Icon name="ai" /> <Icon name="ai" className={SidebarStyles.IconMargin} />
<Text> <Text>
{c("{0} is a date/time and {1} is a person's name").jt`${( {c("{0} is a date/time and {1} is a person's name").jt`${(
<DateTime <DateTime
...@@ -35,8 +41,8 @@ export const QuestionDetails = ({ question }: { question: Question }) => { ...@@ -35,8 +41,8 @@ export const QuestionDetails = ({ question }: { question: Question }) => {
</Flex> </Flex>
)} )}
<Flex gap="sm" align="center"> <Flex gap="sm" align="top">
<Icon name="pencil" /> <Icon name="pencil" className={SidebarStyles.IconMargin} />
<Text> <Text>
{c("{0} is a date/time and {1} is a person's name").jt`${( {c("{0} is a date/time and {1} is a person's name").jt`${(
<DateTime unit="day" value={createdAt} key="date" /> <DateTime unit="day" value={createdAt} key="date" />
...@@ -45,9 +51,20 @@ export const QuestionDetails = ({ question }: { question: Question }) => { ...@@ -45,9 +51,20 @@ export const QuestionDetails = ({ question }: { question: Question }) => {
</Flex> </Flex>
</SidesheetCardSection> </SidesheetCardSection>
<SidesheetCardSection title={t`Saved in`}> <SidesheetCardSection title={t`Saved in`}>
<Flex gap="sm" align="center"> <Flex gap="sm" align="top" color="var(--mb-color-brand)">
<Icon name="folder" /> <Icon
<Text>{question.collection()?.name}</Text> name="folder"
color="var(--mb-color-brand)"
className={SidebarStyles.IconMargin}
/>
<Text>
<Link
to={`/collection/${question.collection()?.id}`}
variant="brand"
>
{question.collection()?.name}
</Link>
</Text>
</Flex> </Flex>
</SidesheetCardSection> </SidesheetCardSection>
<SharingDisplay question={question} /> <SharingDisplay question={question} />
...@@ -58,21 +75,40 @@ export const QuestionDetails = ({ question }: { question: Question }) => { ...@@ -58,21 +75,40 @@ export const QuestionDetails = ({ question }: { question: Question }) => {
function SourceDisplay({ question }: { question: Question }) { function SourceDisplay({ question }: { question: Question }) {
const sourceInfo = question.legacyQueryTable(); const sourceInfo = question.legacyQueryTable();
const metadata = useSelector(getMetadata);
if (!sourceInfo) { if (!sourceInfo) {
return null; return null;
} }
const model = String(sourceInfo.id).includes("card__") ? "card" : "table";
const sourceUrl =
model === "card"
? Urls.browseDatabase(sourceInfo.db as Database)
: getTableUrl(sourceInfo, metadata);
return ( return (
<SidesheetCardSection title={t`Based on`}> <SidesheetCardSection title={t`Based on`}>
<Flex gap="sm" align="center"> <Flex gap="sm" align="center">
{sourceInfo.db && ( {sourceInfo.db && (
<> <>
<Text>{sourceInfo.db.name}</Text> <Text>
<Link
to={`/browse/databases/${sourceInfo.db.id}`}
variant="brand"
>
{sourceInfo.db.name}
</Link>
</Text>
{"/"} {"/"}
</> </>
)} )}
<Text>{sourceInfo?.display_name}</Text> <Text>
<Link to={sourceUrl} variant="brand">
{sourceInfo?.display_name}
</Link>
</Text>
</Flex> </Flex>
</SidesheetCardSection> </SidesheetCardSection>
); );
...@@ -102,7 +138,7 @@ function SharingDisplay({ question }: { question: Question }) { ...@@ -102,7 +138,7 @@ function SharingDisplay({ question }: { question: Question }) {
className={cx( className={cx(
Styles.cursorPointer, Styles.cursorPointer,
Styles.textBrandHover, Styles.textBrandHover,
SidebarStyles.LinkIcon, SidebarStyles.IconMargin,
)} )}
/> />
} }
......
...@@ -13,6 +13,6 @@ ...@@ -13,6 +13,6 @@
padding: 1px; padding: 1px;
} }
.LinkIcon { .IconMargin {
margin-top: 0.2rem; margin-top: 0.25rem;
} }
import { useState } from "react"; import { useMemo, useState } from "react";
import { useMount } from "react-use"; import { useMount } from "react-use";
import { t } from "ttag"; import { t } from "ttag";
import { isInstanceAnalyticsCollection } from "metabase/collections/utils";
import { import {
Sidesheet, Sidesheet,
SidesheetCard, SidesheetCard,
...@@ -40,6 +41,11 @@ export const QuestionInfoSidebar = ({ ...@@ -40,6 +41,11 @@ export const QuestionInfoSidebar = ({
} }
}; };
const isIAQuestion = useMemo(
() => isInstanceAnalyticsCollection(question.collection()),
[question],
);
const dispatch = useDispatch(); const dispatch = useDispatch();
const handleClose = () => dispatch(onCloseQuestionInfo()); const handleClose = () => dispatch(onCloseQuestionInfo());
...@@ -59,14 +65,15 @@ export const QuestionInfoSidebar = ({ ...@@ -59,14 +65,15 @@ export const QuestionInfoSidebar = ({
isOpen={isOpen} isOpen={isOpen}
removeBodyPadding removeBodyPadding
data-testid="question-info-sidebar" data-testid="question-info-sidebar"
size="md"
> >
<Tabs <Tabs
defaultValue="overview" defaultValue="overview"
className={SidesheetStyles.FlexScrollContainer} className={SidesheetStyles.FlexScrollContainer}
> >
<Tabs.List mx="lg"> <Tabs.List mx="xl">
<Tabs.Tab value="overview">{t`Overview`}</Tabs.Tab> <Tabs.Tab value="overview">{t`Overview`}</Tabs.Tab>
<Tabs.Tab value="history">{t`History`}</Tabs.Tab> {!isIAQuestion && <Tabs.Tab value="history">{t`History`}</Tabs.Tab>}
</Tabs.List> </Tabs.List>
<SidesheetTabPanelContainer> <SidesheetTabPanelContainer>
<Tabs.Panel value="overview"> <Tabs.Panel value="overview">
......
...@@ -14,9 +14,9 @@ const setupEnterprise = (opts: SetupOpts) => { ...@@ -14,9 +14,9 @@ const setupEnterprise = (opts: SetupOpts) => {
}); });
}; };
describe("QuestionInfoSidebar", () => { describe("QuestionInfoSidebar > enterprise", () => {
describe("moderation reviews", () => { describe("moderation reviews", () => {
it("should not show the verification badge", async () => { it("should not show the verification badge without content verification feature", async () => {
const card = createMockCard({ const card = createMockCard({
moderation_reviews: [ moderation_reviews: [
createMockModerationReview({ status: "verified" }), createMockModerationReview({ status: "verified" }),
......
import { screen } from "__support__/ui";
import {
createMockCard,
createMockCollection,
createMockModerationReview,
createMockSettings,
createMockTokenFeatures,
} from "metabase-types/api/mocks";
import type { SetupOpts } from "./setup";
import { setup } from "./setup";
const setupEnterprise = (opts: SetupOpts) => {
return setup({
...opts,
settings: createMockSettings({
"token-features": createMockTokenFeatures({
content_verification: true,
cache_granular_controls: true,
audit_app: true,
}),
}),
hasEnterprisePlugins: true,
});
};
describe("QuestionInfoSidebar > premium", () => {
describe("content verification", () => {
it("should show the verification badge for verified content", async () => {
const card = createMockCard({
moderation_reviews: [
createMockModerationReview({ status: "verified" }),
],
});
await setupEnterprise({ card });
expect(screen.getByText(/verified this/)).toBeInTheDocument();
});
it("should not show the verification badge for unverified content", async () => {
const card = createMockCard();
await setupEnterprise({ card });
expect(screen.queryByText(/verified this/)).not.toBeInTheDocument();
});
});
describe("analytics content", () => {
it("should show the history section for non analytics content", async () => {
await setupEnterprise({
card: createMockCard({
collection: createMockCollection(),
}),
});
expect(await screen.findByText("History")).toBeInTheDocument();
});
});
it("should not show the history section for instance analytics question", async () => {
await setupEnterprise({
card: createMockCard({
collection: createMockCollection({ type: "instance-analytics" }),
}),
});
expect(screen.queryByText("History")).not.toBeInTheDocument();
});
});
...@@ -3,6 +3,7 @@ import { useMount } from "react-use"; ...@@ -3,6 +3,7 @@ import { useMount } from "react-use";
import { match } from "ts-pattern"; import { match } from "ts-pattern";
import { t } from "ttag"; import { t } from "ttag";
import { isInstanceAnalyticsCollection } from "metabase/collections/utils";
import { Sidesheet, SidesheetCard } from "metabase/common/components/Sidesheet"; import { Sidesheet, SidesheetCard } from "metabase/common/components/Sidesheet";
import { useDispatch } from "metabase/lib/redux"; import { useDispatch } from "metabase/lib/redux";
import { PLUGIN_CACHING, PLUGIN_MODEL_PERSISTENCE } from "metabase/plugins"; import { PLUGIN_CACHING, PLUGIN_MODEL_PERSISTENCE } from "metabase/plugins";
...@@ -84,6 +85,12 @@ export const QuestionSettingsSidebar = ({ ...@@ -84,6 +85,12 @@ export const QuestionSettingsSidebar = ({
}; };
export const shouldShowQuestionSettingsSidebar = (question: Question) => { export const shouldShowQuestionSettingsSidebar = (question: Question) => {
const isIAQuestion = isInstanceAnalyticsCollection(question.collection());
if (isIAQuestion) {
return false;
}
const isCacheableQuestion = const isCacheableQuestion =
PLUGIN_CACHING.isGranularCachingEnabled() && PLUGIN_CACHING.isGranularCachingEnabled() &&
PLUGIN_CACHING.hasQuestionCacheSection(question); PLUGIN_CACHING.hasQuestionCacheSection(question);
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment