From e8dc5ec5a66b9aa052e23417cdb9f59d4eab2db4 Mon Sep 17 00:00:00 2001 From: Phoomparin Mano <poom@metabase.com> Date: Mon, 13 May 2024 15:57:16 +0700 Subject: [PATCH] Migrate smart scalar and night mode to support Mantine theming (#42347) * migrate smart scalar to css modules * lighten theme color * adjust margin * map space 1 to xs * use c prop instead of color * use rem unit * use rem unit * add y margin * return null when no children * comment on text variation * cleanup unused components and add dark mode todo * remove todo comment * support night mode in embedding * lift dashboard theme to redux * make theme non-nullable with default * use component prop instead of title tag * optional display theme * restore tooltip state * add default mock state * remove theme from embed * replace handleActions with createReducer --- .../src/embedding-sdk/types/theme/index.ts | 3 + .../src/metabase-types/store/dashboard.ts | 3 + .../metabase-types/store/mocks/dashboard.ts | 1 + .../src/metabase/dashboard/actions/index.ts | 1 + .../src/metabase/dashboard/actions/theme.ts | 6 + frontend/src/metabase/dashboard/constants.ts | 1 + .../dashboard/hoc/DashboardControls.jsx | 34 ++-- frontend/src/metabase/dashboard/reducers.js | 7 + .../metabase/dashboard/reducers.unit.spec.js | 1 + frontend/src/metabase/dashboard/selectors.ts | 7 + frontend/src/metabase/public/lib/analytics.ts | 3 +- frontend/src/metabase/public/lib/types.ts | 4 +- .../SmartScalar/SmartScalar.jsx | 162 ++++++++++++------ .../SmartScalar/SmartScalar.styled.tsx | 79 --------- 14 files changed, 167 insertions(+), 145 deletions(-) create mode 100644 frontend/src/metabase/dashboard/actions/theme.ts diff --git a/enterprise/frontend/src/embedding-sdk/types/theme/index.ts b/enterprise/frontend/src/embedding-sdk/types/theme/index.ts index 48c0eac9f10..20c4f805572 100644 --- a/enterprise/frontend/src/embedding-sdk/types/theme/index.ts +++ b/enterprise/frontend/src/embedding-sdk/types/theme/index.ts @@ -32,6 +32,9 @@ export interface MetabaseColors { /** Text color on light elements. Should be a darker color for readability. */ "text-light"?: string; + + /** Lighter variation of dark text on light elements. */ + "text-medium"?: string; } export type MetabaseColor = keyof MetabaseColors; diff --git a/frontend/src/metabase-types/store/dashboard.ts b/frontend/src/metabase-types/store/dashboard.ts index 4c15ede47fc..4556d184c65 100644 --- a/frontend/src/metabase-types/store/dashboard.ts +++ b/frontend/src/metabase-types/store/dashboard.ts @@ -1,3 +1,4 @@ +import type { DisplayTheme } from "metabase/public/lib/types"; import type { Dashboard, DashboardId, @@ -112,4 +113,6 @@ export interface DashboardState { toastDashboardId: number | null; }; tabDeletions: Record<TabDeletionId, TabDeletion>; + + theme: DisplayTheme | null; } diff --git a/frontend/src/metabase-types/store/mocks/dashboard.ts b/frontend/src/metabase-types/store/mocks/dashboard.ts index aac23fbf4e8..949d208ed25 100644 --- a/frontend/src/metabase-types/store/mocks/dashboard.ts +++ b/frontend/src/metabase-types/store/mocks/dashboard.ts @@ -30,5 +30,6 @@ export const createMockDashboardState = ( toastDashboardId: null, }, tabDeletions: {}, + theme: "light", ...opts, }); diff --git a/frontend/src/metabase/dashboard/actions/index.ts b/frontend/src/metabase/dashboard/actions/index.ts index 92c1894430a..efbd6911fa9 100644 --- a/frontend/src/metabase/dashboard/actions/index.ts +++ b/frontend/src/metabase/dashboard/actions/index.ts @@ -11,3 +11,4 @@ export * from "./sharing"; export * from "./ui"; export * from "./actions"; export * from "./tabs"; +export * from "./theme"; diff --git a/frontend/src/metabase/dashboard/actions/theme.ts b/frontend/src/metabase/dashboard/actions/theme.ts new file mode 100644 index 00000000000..153111b39c2 --- /dev/null +++ b/frontend/src/metabase/dashboard/actions/theme.ts @@ -0,0 +1,6 @@ +import { createAction } from "@reduxjs/toolkit"; + +import type { DisplayTheme } from "metabase/public/lib/types"; + +export const SET_DISPLAY_THEME = "metabase/dashboard/SET_DISPLAY_THEME"; +export const setDisplayTheme = createAction<DisplayTheme>(SET_DISPLAY_THEME); diff --git a/frontend/src/metabase/dashboard/constants.ts b/frontend/src/metabase/dashboard/constants.ts index 7f80cdff14b..3bd0e9293af 100644 --- a/frontend/src/metabase/dashboard/constants.ts +++ b/frontend/src/metabase/dashboard/constants.ts @@ -39,6 +39,7 @@ export const INITIAL_DASHBOARD_STATE: DashboardState = { toastDashboardId: null, }, tabDeletions: {}, + theme: "light", }; export const DASHBOARD_SLOW_TIMEOUT = 15 * 1000; diff --git a/frontend/src/metabase/dashboard/hoc/DashboardControls.jsx b/frontend/src/metabase/dashboard/hoc/DashboardControls.jsx index 74db0ac6f30..082d49b7f4b 100644 --- a/frontend/src/metabase/dashboard/hoc/DashboardControls.jsx +++ b/frontend/src/metabase/dashboard/hoc/DashboardControls.jsx @@ -5,18 +5,29 @@ import { replace } from "react-router-redux"; import screenfull from "screenfull"; import HideS from "metabase/css/core/hide.module.css"; +import { setDisplayTheme } from "metabase/dashboard/actions"; +import { getDisplayTheme } from "metabase/dashboard/selectors"; import * as MetabaseAnalytics from "metabase/lib/analytics"; import { parseHashOptions, stringifyHashOptions } from "metabase/lib/browser"; const TICK_PERIOD = 1; // seconds +const mapStateToProps = (state, props) => ({ + theme: getDisplayTheme(state), +}); + +const mapDispatchToProps = { replace, setDisplayTheme }; + /* This contains some state for dashboard controls on both private and embedded dashboards. * It should probably be in Redux? * * @deprecated HOCs are deprecated */ export const DashboardControls = ComposedComponent => - connect(null, { replace })( + connect( + mapStateToProps, + mapDispatchToProps, + )( class extends Component { static displayName = "DashboardControls[" + @@ -25,10 +36,7 @@ export const DashboardControls = ComposedComponent => state = { isFullscreen: false, - theme: null, - refreshPeriod: null, - hideParameters: null, }; @@ -71,9 +79,10 @@ export const DashboardControls = ComposedComponent => ? null : options.refresh, ); - this.setTheme(options.theme); + this.setFullscreen(options.fullscreen); this.setHideParameters(options.hide_parameters); + this.props.setDisplayTheme(options.theme); }; syncUrlHashToState() { @@ -82,7 +91,7 @@ export const DashboardControls = ComposedComponent => const { refresh, fullscreen, theme } = parseHashOptions(location.hash); this.setRefreshPeriod(refresh); this.setFullscreen(fullscreen); - this.setTheme(theme); + this.props.setDisplayTheme(theme); } syncStateToUrlHash = () => { @@ -98,7 +107,7 @@ export const DashboardControls = ComposedComponent => }; setValue("refresh", this.state.refreshPeriod); setValue("fullscreen", this.state.isFullscreen); - setValue("theme", this.state.theme); + setValue("theme", this.props.theme); delete options.night; // DEPRECATED: options.night @@ -143,12 +152,7 @@ export const DashboardControls = ComposedComponent => // Preserve existing behavior, while keeping state in a new `theme` key setNightMode = isNightMode => { - const theme = isNightMode ? "night" : null; - this.setState({ theme }); - }; - - setTheme = theme => { - this.setState({ theme }); + this.props.setDisplayTheme(isNightMode ? "night" : null); }; setFullscreen = async (isFullscreen, browserFullscreen = true) => { @@ -236,8 +240,8 @@ export const DashboardControls = ComposedComponent => <ComposedComponent {...this.props} {...this.state} - isNightMode={this.state.theme === "night"} - hasNightModeToggle={this.state.theme !== "transparent"} + isNightMode={this.props.theme === "night"} + hasNightModeToggle={this.props.theme !== "transparent"} setRefreshElapsedHook={this.setRefreshElapsedHook} loadDashboardParams={this.loadDashboardParams} onNightModeChange={this.setNightMode} diff --git a/frontend/src/metabase/dashboard/reducers.js b/frontend/src/metabase/dashboard/reducers.js index 1a993d4fd41..10e0c74edb4 100644 --- a/frontend/src/metabase/dashboard/reducers.js +++ b/frontend/src/metabase/dashboard/reducers.js @@ -1,3 +1,4 @@ +import { createReducer } from "@reduxjs/toolkit"; import { assoc, dissoc, assocIn, updateIn, chain, merge } from "icepick"; import produce from "immer"; import reduceReducers from "reduce-reducers"; @@ -45,6 +46,7 @@ import { SHOW_AUTO_APPLY_FILTERS_TOAST, tabsReducer, FETCH_CARD_DATA_PENDING, + SET_DISPLAY_THEME, fetchDashboard, } from "./actions"; import { INITIAL_DASHBOARD_STATE } from "./constants"; @@ -515,6 +517,10 @@ const autoApplyFilters = handleActions( INITIAL_DASHBOARD_STATE.autoApplyFilters, ); +const theme = createReducer(INITIAL_DASHBOARD_STATE.theme, builder => { + builder.addCase(SET_DISPLAY_THEME, (state, { payload }) => payload || null); +}); + export const dashboardReducers = reduceReducers( INITIAL_DASHBOARD_STATE, combineReducers({ @@ -536,6 +542,7 @@ export const dashboardReducers = reduceReducers( // Combined reducer needs to init state for every slice selectedTabId: (state = INITIAL_DASHBOARD_STATE.selectedTabId) => state, tabDeletions: (state = INITIAL_DASHBOARD_STATE.tabDeletions) => state, + theme, }), tabsReducer, ); diff --git a/frontend/src/metabase/dashboard/reducers.unit.spec.js b/frontend/src/metabase/dashboard/reducers.unit.spec.js index 530f54a8eb8..7427b2d6a7c 100644 --- a/frontend/src/metabase/dashboard/reducers.unit.spec.js +++ b/frontend/src/metabase/dashboard/reducers.unit.spec.js @@ -48,6 +48,7 @@ describe("dashboard reducers", () => { toastDashboardId: null, }, tabDeletions: {}, + theme: "light", }); }); diff --git a/frontend/src/metabase/dashboard/selectors.ts b/frontend/src/metabase/dashboard/selectors.ts index ba47a75086c..8c636422727 100644 --- a/frontend/src/metabase/dashboard/selectors.ts +++ b/frontend/src/metabase/dashboard/selectors.ts @@ -464,3 +464,10 @@ export const getParameterMappingsBeforeEditing = createSelector( return map; }, ); + +export const getDisplayTheme = (state: State) => state.dashboard.theme; + +export const getIsNightMode = createSelector( + [getDisplayTheme], + theme => theme === "night", +); diff --git a/frontend/src/metabase/public/lib/analytics.ts b/frontend/src/metabase/public/lib/analytics.ts index 66b410dd5e7..c29e0f61644 100644 --- a/frontend/src/metabase/public/lib/analytics.ts +++ b/frontend/src/metabase/public/lib/analytics.ts @@ -3,6 +3,7 @@ import { trackSchemaEvent } from "metabase/lib/analytics"; import type { EmbeddingDisplayOptions, + DisplayTheme, EmbedResource, EmbedResourceType, } from "./types"; @@ -13,7 +14,7 @@ const SCHEMA_VERSION = "1-0-1"; type Appearance = { titled: boolean; bordered: boolean; - theme: "light" | "night" | "transparent"; + theme: DisplayTheme; font: "instance" | "custom"; hide_download_button: boolean | null; }; diff --git a/frontend/src/metabase/public/lib/types.ts b/frontend/src/metabase/public/lib/types.ts index 0b2e34ae220..a1332af1cc6 100644 --- a/frontend/src/metabase/public/lib/types.ts +++ b/frontend/src/metabase/public/lib/types.ts @@ -1,5 +1,7 @@ import type { Card, Dashboard } from "metabase-types/api"; +export type DisplayTheme = "light" | "night" | "transparent"; + export type EmbedModalStep = "application" | "legalese" | null; export type EmbedResource = (Card | Dashboard) & { @@ -25,7 +27,7 @@ export type EmbeddingParametersValues = Record<string, string>; export type EmbeddingDisplayOptions = { font: null | string; - theme: "light" | "night" | "transparent"; + theme: DisplayTheme; bordered: boolean; titled: boolean; hide_download_button: boolean | null; diff --git a/frontend/src/metabase/visualizations/visualizations/SmartScalar/SmartScalar.jsx b/frontend/src/metabase/visualizations/visualizations/SmartScalar/SmartScalar.jsx index 2eba978f71f..29ac5419c77 100644 --- a/frontend/src/metabase/visualizations/visualizations/SmartScalar/SmartScalar.jsx +++ b/frontend/src/metabase/visualizations/visualizations/SmartScalar/SmartScalar.jsx @@ -7,11 +7,14 @@ import { t, jt } from "ttag"; import { Ellipsified } from "metabase/core/components/Ellipsified"; import Tooltip from "metabase/core/components/Tooltip"; import DashboardS from "metabase/css/dashboard.module.css"; -import { color } from "metabase/lib/colors"; +import { getIsNightMode } from "metabase/dashboard/selectors"; +import { color, lighten } from "metabase/lib/colors"; import { formatValue } from "metabase/lib/formatting/value"; import { measureTextWidth } from "metabase/lib/measure-text"; +import { useSelector } from "metabase/lib/redux"; import { isEmpty } from "metabase/lib/validate"; import EmbedFrameS from "metabase/public/components/EmbedFrame/EmbedFrame.module.css"; +import { Box, Flex, Title, Text, useMantineTheme } from "metabase/ui"; import ScalarValue, { ScalarWrapper, } from "metabase/visualizations/components/ScalarValue"; @@ -28,18 +31,7 @@ import { import { ScalarContainer } from "../Scalar/Scalar.styled"; import { SmartScalarComparisonWidget } from "./SettingsComponents/SmartScalarSettingsWidgets"; -import { - PreviousValueDetails, - VariationContainer, - PreviousValueWrapper, - PreviousValueNumber, - Separator, - Variation, - VariationIcon, - VariationContainerTooltip, - VariationValue, - ScalarPeriodContent, -} from "./SmartScalar.styled"; +import { VariationIcon, VariationValue } from "./SmartScalar.styled"; import { computeTrend, CHANGE_TYPE_OPTIONS } from "./compute"; import { DASHCARD_HEADER_HEIGHT, @@ -137,27 +129,30 @@ export function SmartScalar({ /> </span> </ScalarContainer> - {isPeriodVisible(innerHeight) && ( - <ScalarPeriod lines={1} period={display.date} /> - )} + {isPeriodVisible(innerHeight) && <ScalarPeriod period={display.date} />} {comparisons.map((comparison, index) => ( - <PreviousValueWrapper key={index} data-testid="scalar-previous-value"> + <Box maw="100%" key={index} data-testid="scalar-previous-value"> <PreviousValueComparison comparison={comparison} fontFamily={fontFamily} formatOptions={formatOptions} width={width} /> - </PreviousValueWrapper> + </Box> ))} </ScalarWrapper> ); } -function ScalarPeriod({ lines = 2, period, onClick }) { +function ScalarPeriod({ period, onClick }) { return ( - <ScalarTitleContainer data-testid="scalar-period" lines={lines}> - <ScalarPeriodContent + <ScalarTitleContainer data-testid="scalar-period" lines={1}> + <Text + component="h3" + ta="center" + style={{ overflow: "hidden", cursor: onClick && "pointer" }} + fw={700} + size="0.875rem" className={cx( DashboardS.fullscreenNormalText, DashboardS.fullscreenNightText, @@ -165,14 +160,36 @@ function ScalarPeriod({ lines = 2, period, onClick }) { )} onClick={onClick} > - <Ellipsified tooltip={period} lines={lines} placement="bottom"> + <Ellipsified tooltip={period} lines={1} placement="bottom"> {period} </Ellipsified> - </ScalarPeriodContent> + </Text> </ScalarTitleContainer> ); } +const Separator = ({ inTooltip }) => { + const theme = useMantineTheme(); + const isNightMode = useSelector(getIsNightMode); + + const separatorColor = + isNightMode || inTooltip + ? lighten(theme.fn.themeColor("text-medium"), 0.15) + : lighten(theme.fn.themeColor("text-light"), 0.25); + + return ( + <Text + display="inline-block" + mx="0.2rem" + style={{ transform: "scale(0.7)" }} + c={separatorColor} + span + > + {" • "} + </Text> + ); +}; + function PreviousValueComparison({ comparison, width, @@ -191,6 +208,9 @@ function PreviousValueComparison({ display, } = comparison; + const theme = useMantineTheme(); + const isNightMode = useSelector(getIsNightMode); + const fittedChangeDisplay = changeType === CHANGE_TYPE_OPTIONS.CHANGED.CHANGE_TYPE ? formatChangeAutoPrecision(percentChange, { @@ -199,13 +219,13 @@ function PreviousValueComparison({ width: getChangeWidth(width), }) : display.percentChange; - const separator = <Separator> • </Separator>; + const availableComparisonWidth = width - 4 * SPACING - ICON_SIZE - ICON_MARGIN_RIGHT - - measureTextWidth(innerText(separator), { + measureTextWidth(innerText(<Separator />), { size: fontSize, family: fontFamily, weight: 700, @@ -223,21 +243,35 @@ function PreviousValueComparison({ : []), "", ]; - const detailCandidates = valueCandidates.map(valueStr => { + + const getDetailCandidate = (valueStr, { inTooltip } = {}) => { if (isEmpty(valueStr)) { return comparisonDescStr; } + const descColor = + isNightMode || inTooltip + ? lighten(theme.fn.themeColor("text-medium"), 0.45) + : "text-light"; + if (isEmpty(comparisonDescStr)) { return ( - <PreviousValueNumber key={valueStr}>{valueStr}</PreviousValueNumber> + <Text key={valueStr} c={descColor} span> + {valueStr} + </Text> ); } return jt`${comparisonDescStr}: ${( - <PreviousValueNumber key="value-str">{valueStr}</PreviousValueNumber> + <Text key="value-str" c={descColor} span> + {valueStr} + </Text> )}`; - }); + }; + + const detailCandidates = valueCandidates.map(valueStr => + getDetailCandidate(valueStr), + ); const fullDetailDisplay = detailCandidates[0]; const fittedDetailDisplay = detailCandidates.find( e => @@ -248,36 +282,66 @@ function PreviousValueComparison({ }) <= availableComparisonWidth, ); - const VariationPercent = ({ iconSize, children }) => ( - <Variation color={changeColor}> - {changeArrowIconName && ( - <VariationIcon name={changeArrowIconName} size={iconSize} /> - )} - <VariationValue showTooltip={false}>{children}</VariationValue> - </Variation> - ); - const VariationDetails = ({ children }) => - children ? ( - <PreviousValueDetails> - {separator} + const tooltipFullDetailDisplay = getDetailCandidate(valueCandidates[0], { + inTooltip: true, + }); + + const VariationPercent = ({ inTooltip, iconSize, children }) => { + const noChangeColor = + inTooltip || isNightMode + ? lighten(theme.fn.themeColor("text-medium"), 0.3) + : "text-light"; + + return ( + <Flex align="center" maw="100%" c={changeColor ?? noChangeColor}> + {changeArrowIconName && ( + <VariationIcon name={changeArrowIconName} size={iconSize} /> + )} + <VariationValue showTooltip={false}>{children}</VariationValue> + </Flex> + ); + }; + + const VariationDetails = ({ inTooltip, children }) => { + if (!children) { + return null; + } + + const detailColor = + isNightMode || inTooltip + ? lighten(theme.fn.themeColor("text-light"), 0.25) + : "text-medium"; + + return ( + <Title order={4} c={detailColor} style={{ whiteSpace: "pre" }}> + <Separator inTooltip={inTooltip} /> {children} - </PreviousValueDetails> - ) : null; + </Title> + ); + }; return ( <Tooltip isEnabled={fullDetailDisplay !== fittedDetailDisplay} placement="bottom" tooltip={ - <VariationContainerTooltip className="variation-container-tooltip"> - <VariationPercent iconSize={TOOLTIP_ICON_SIZE}> + <Flex align="center"> + <VariationPercent iconSize={TOOLTIP_ICON_SIZE} inTooltip> {display.percentChange} </VariationPercent> - <VariationDetails>{fullDetailDisplay}</VariationDetails> - </VariationContainerTooltip> + <VariationDetails inTooltip> + {tooltipFullDetailDisplay} + </VariationDetails> + </Flex> } > - <VariationContainer + <Flex + wrap="wrap" + align="center" + justify="center" + mx="sm" + my="xs" + lh="1.2rem" className={cx( DashboardS.fullscreenNormalText, DashboardS.fullscreenNightText, @@ -288,7 +352,7 @@ function PreviousValueComparison({ {fittedChangeDisplay} </VariationPercent> <VariationDetails>{fittedDetailDisplay}</VariationDetails> - </VariationContainer> + </Flex> </Tooltip> ); } diff --git a/frontend/src/metabase/visualizations/visualizations/SmartScalar/SmartScalar.styled.tsx b/frontend/src/metabase/visualizations/visualizations/SmartScalar/SmartScalar.styled.tsx index 4df0ac91c9e..b8a5de15a7b 100644 --- a/frontend/src/metabase/visualizations/visualizations/SmartScalar/SmartScalar.styled.tsx +++ b/frontend/src/metabase/visualizations/visualizations/SmartScalar/SmartScalar.styled.tsx @@ -1,27 +1,9 @@ import styled from "@emotion/styled"; import { Ellipsified } from "metabase/core/components/Ellipsified"; -import DashboardS from "metabase/css/dashboard.module.css"; -import { color, lighten } from "metabase/lib/colors"; -import { isEmpty } from "metabase/lib/validate"; import { space } from "metabase/styled-components/theme"; import { Icon } from "metabase/ui"; -export const Variation = styled.div` - color: ${props => (isEmpty(props.color) ? color("text-light") : props.color)}; - display: flex; - align-items: center; - max-width: 100%; - - .${DashboardS.Dashboard}.${DashboardS.DashboardNight}.${DashboardS.DashboardFullscreen} - .${DashboardS.fullscreenNightText} - &, - .variation-container-tooltip & { - color: ${props => - isEmpty(props.color) ? lighten("text-medium", 0.3) : props.color}; - } -`; - export const VariationIcon = styled(Icon)` display: flex; align-items: center; @@ -33,64 +15,3 @@ export const VariationIcon = styled(Icon)` export const VariationValue = styled(Ellipsified)` font-weight: 900; `; - -export const VariationContainerTooltip = styled.div` - display: flex; - align-items: center; -`; - -export const PreviousValueWrapper = styled.div` - max-width: 100%; -`; - -export const VariationContainer = styled.div` - align-items: center; - display: flex; - flex-wrap: wrap; - justify-content: center; - margin: ${space(0)} ${space(1)}; - line-height: 1.2rem; -`; - -export const Separator = styled.span` - display: inline-block; - transform: scale(0.7); - margin: 0 0.2rem; - color: ${lighten("text-light", 0.25)}; - - .${DashboardS.Dashboard}.${DashboardS.DashboardNight}.${DashboardS.DashboardFullscreen} - .${DashboardS.fullscreenNightText} - &, - .variation-container-tooltip & { - color: ${lighten("text-medium", 0.15)}; - } -`; - -export const PreviousValueDetails = styled.h4` - color: ${color("text-medium")}; - white-space: pre; - - .${DashboardS.Dashboard}.${DashboardS.DashboardNight}.${DashboardS.DashboardFullscreen} - .${DashboardS.fullscreenNightText} - &, - .variation-container-tooltip & { - color: ${lighten("text-light", 0.25)}; - } -`; - -export const PreviousValueNumber = styled.span` - color: ${color("text-light")}; - - .${DashboardS.Dashboard}.${DashboardS.DashboardNight}.${DashboardS.DashboardFullscreen} - .${DashboardS.fullscreenNightText} - &, - .variation-container-tooltip & { - color: ${lighten("text-medium", 0.45)}; - } -`; -export const ScalarPeriodContent = styled.h3` - text-align: center; - overflow: hidden; - cursor: ${props => props.onClick && "pointer"}; - font-size: 0.875rem; -`; -- GitLab