mirror of
https://github.com/apache/superset.git
synced 2026-05-28 11:15:24 +00:00
feat(dashboard): live theme preview in ThemeSelectorModal
When the modal is open, the targeted component re-renders with the candidate theme as soon as the user picks an option — no dashboard- dirty round-trip required. Cancel reverts; Apply commits to Redux. Implementation: - New \`previewThemeStore\` (module-level subscribable map keyed by layoutId, distinguishing "no preview" / "preview value=null" / "preview value=number"). Tiny surface: \`set\`/\`clear\`/\`get\`/ \`subscribe\`. No-op \`set\` / \`clear\` calls don't fire listeners. - \`useEffectiveThemeId\` now subscribes via \`useSyncExternalStore\` and prefers the preview value over the Redux-resolved id when present. - \`ThemeSelectorModal\` writes the in-flight selection through the store as the user picks options; cleanup on close (Cancel, X button, escape) clears it. Apply dispatches the Redux action *before* hiding, so the post-cleanup re-resolution lands on the saved value (no flicker). Snapshot of the resolved id at open-time goes through a \`useRef\` because \`currentThemeId\` itself becomes reactive (it would already reflect the in-flight preview), so we can't read it for "what should we revert to?". 6 new tests for the preview store: get-undefined-for-unknown, set-stores-numeric, set-stores-explicit-null, clear-removes, subscriber-fires-on-real-change-and-not-on-no-op, multi-layoutId- independence. Total dashboard ComponentThemeProvider suite is now 14 passing tests. Drops "live preview" from the deferred-items list in SIP.md. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
8
SIP.md
8
SIP.md
@@ -100,7 +100,12 @@ together rather than as separate churn passes.
|
||||
Modal triggered from "Apply theme" in `ComponentHeaderControls`. Shows:
|
||||
- A theme picker populated from the CRUD `/api/v1/theme/` endpoint (same source the dashboard-level picker uses).
|
||||
- A "Clear override (inherit)" button when `themeId` is already set.
|
||||
- Preview is **deferred** — initial scope is just save/cancel.
|
||||
- **Live preview**: as the user picks options the targeted component
|
||||
re-renders with the candidate theme tokens immediately, *without*
|
||||
marking the dashboard dirty. Cancel reverts; Apply commits to Redux.
|
||||
Implemented via a tiny module-level subscribable `previewThemeStore`
|
||||
+ `useSyncExternalStore` in `useEffectiveThemeId` (preview wins over
|
||||
the Redux-resolved id when present).
|
||||
|
||||
On save it dispatches a Redux action that updates the component's `meta.themeId` and marks the dashboard dirty.
|
||||
|
||||
@@ -155,7 +160,6 @@ Each phase brings its own tests; the cumulative bar:
|
||||
## Out-of-scope (potential follow-ups)
|
||||
|
||||
- **Theme presets** — "apply this theme to all charts of viz type X" via a dashboard-level rule.
|
||||
- **Live preview** in `ThemeSelectorModal`.
|
||||
- **Theme inheritance debugger** — devtools view that shows which level set each token for a hovered component.
|
||||
- **Bulk operations** — multi-select components and apply a theme to all.
|
||||
|
||||
|
||||
@@ -0,0 +1,70 @@
|
||||
/**
|
||||
* 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 { previewThemeStore } from './previewThemeStore';
|
||||
|
||||
afterEach(() => {
|
||||
// Defensive — module-level state would leak between tests otherwise.
|
||||
previewThemeStore.clear('CHART-a');
|
||||
previewThemeStore.clear('CHART-b');
|
||||
});
|
||||
|
||||
test('get returns undefined for unknown layoutId', () => {
|
||||
expect(previewThemeStore.get('CHART-a')).toBeUndefined();
|
||||
});
|
||||
|
||||
test('set stores a numeric preview readable by get', () => {
|
||||
previewThemeStore.set('CHART-a', 7);
|
||||
expect(previewThemeStore.get('CHART-a')).toBe(7);
|
||||
});
|
||||
|
||||
test('set stores explicit null (distinct from "no preview")', () => {
|
||||
previewThemeStore.set('CHART-a', null);
|
||||
expect(previewThemeStore.get('CHART-a')).toBeNull();
|
||||
// Distinct from the unknown-key case
|
||||
expect(previewThemeStore.get('CHART-b')).toBeUndefined();
|
||||
});
|
||||
|
||||
test('clear removes the entry; subsequent get returns undefined', () => {
|
||||
previewThemeStore.set('CHART-a', 7);
|
||||
previewThemeStore.clear('CHART-a');
|
||||
expect(previewThemeStore.get('CHART-a')).toBeUndefined();
|
||||
});
|
||||
|
||||
test('subscribers fire on set and clear, do not fire on no-op set', () => {
|
||||
const listener = jest.fn();
|
||||
const unsubscribe = previewThemeStore.subscribe(listener);
|
||||
previewThemeStore.set('CHART-a', 7);
|
||||
previewThemeStore.set('CHART-a', 7); // no-op (same value)
|
||||
previewThemeStore.set('CHART-a', 9);
|
||||
previewThemeStore.clear('CHART-a');
|
||||
previewThemeStore.clear('CHART-a'); // no-op
|
||||
unsubscribe();
|
||||
previewThemeStore.set('CHART-a', 1); // not observed (unsubscribed)
|
||||
expect(listener).toHaveBeenCalledTimes(3);
|
||||
});
|
||||
|
||||
test('multiple layoutIds are tracked independently', () => {
|
||||
previewThemeStore.set('CHART-a', 1);
|
||||
previewThemeStore.set('CHART-b', 2);
|
||||
expect(previewThemeStore.get('CHART-a')).toBe(1);
|
||||
expect(previewThemeStore.get('CHART-b')).toBe(2);
|
||||
previewThemeStore.clear('CHART-a');
|
||||
expect(previewThemeStore.get('CHART-a')).toBeUndefined();
|
||||
expect(previewThemeStore.get('CHART-b')).toBe(2);
|
||||
});
|
||||
@@ -0,0 +1,74 @@
|
||||
/**
|
||||
* 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.
|
||||
*/
|
||||
|
||||
/**
|
||||
* Module-level subscribable store for transient per-component theme
|
||||
* previews. Used by `ThemeSelectorModal` to make a draft selection
|
||||
* visually applied without committing to Redux (which would mark the
|
||||
* dashboard dirty). `ComponentThemeProvider` subscribes via
|
||||
* `useSyncExternalStore` and prefers a present preview over the
|
||||
* resolved-from-Redux `themeId`.
|
||||
*
|
||||
* `null` means "explicitly clear the override during preview" — the
|
||||
* provider treats it the same way it treats a Redux `null`. Absence
|
||||
* (key not in the map) means "no preview active; use Redux".
|
||||
*/
|
||||
|
||||
type PreviewValue = number | null;
|
||||
type Listener = () => void;
|
||||
|
||||
const previewMap = new Map<string, PreviewValue>();
|
||||
const listeners = new Set<Listener>();
|
||||
|
||||
const emit = (): void => {
|
||||
listeners.forEach(l => l());
|
||||
};
|
||||
|
||||
export const previewThemeStore = {
|
||||
/** Sets a transient preview for `layoutId`. Replaces any prior preview. */
|
||||
set(layoutId: string, themeId: PreviewValue): void {
|
||||
if (previewMap.get(layoutId) === themeId) return;
|
||||
previewMap.set(layoutId, themeId);
|
||||
emit();
|
||||
},
|
||||
|
||||
/** Clears any preview for `layoutId`. No-op when none is active. */
|
||||
clear(layoutId: string): void {
|
||||
if (!previewMap.has(layoutId)) return;
|
||||
previewMap.delete(layoutId);
|
||||
emit();
|
||||
},
|
||||
|
||||
/**
|
||||
* Returns the previewed value for `layoutId`, or `undefined` when no
|
||||
* preview is active. Used by `ComponentThemeProvider` via
|
||||
* `useSyncExternalStore`. Returning `undefined` (vs `null`) lets
|
||||
* callers distinguish "no preview" from "preview the cleared state".
|
||||
*/
|
||||
get(layoutId: string): PreviewValue | undefined {
|
||||
return previewMap.has(layoutId) ? previewMap.get(layoutId) : undefined;
|
||||
},
|
||||
|
||||
subscribe(listener: Listener): () => void {
|
||||
listeners.add(listener);
|
||||
return () => {
|
||||
listeners.delete(listener);
|
||||
};
|
||||
},
|
||||
};
|
||||
@@ -16,9 +16,11 @@
|
||||
* specific language governing permissions and limitations
|
||||
* under the License.
|
||||
*/
|
||||
import { useSyncExternalStore } from 'react';
|
||||
import { useSelector } from 'react-redux';
|
||||
import type { DashboardLayout, RootState } from 'src/dashboard/types';
|
||||
import { DASHBOARD_ROOT_ID } from 'src/dashboard/util/constants';
|
||||
import { previewThemeStore } from './previewThemeStore';
|
||||
|
||||
/**
|
||||
* Walks up the dashboard layout tree from `layoutId` and returns the first
|
||||
@@ -59,11 +61,22 @@ export function pickEffectiveThemeId(
|
||||
* Redux hook variant of `pickEffectiveThemeId`. Memoizes on the layout
|
||||
* reference; consumers that only care about the resolved id (not the layout
|
||||
* map itself) won't re-render when sibling components change their meta.
|
||||
*
|
||||
* If `ThemeSelectorModal` has registered a preview override for this
|
||||
* `layoutId` via `previewThemeStore`, the preview wins — that's how the
|
||||
* modal applies a draft selection visually without committing to Redux.
|
||||
*/
|
||||
export function useEffectiveThemeId(
|
||||
layoutId: string | undefined,
|
||||
): number | null {
|
||||
return useSelector<RootState, number | null>(state =>
|
||||
const reduxResolved = useSelector<RootState, number | null>(state =>
|
||||
pickEffectiveThemeId(layoutId, state.dashboardLayout?.present),
|
||||
);
|
||||
const preview = useSyncExternalStore(
|
||||
previewThemeStore.subscribe,
|
||||
() =>
|
||||
layoutId === undefined ? undefined : previewThemeStore.get(layoutId),
|
||||
() => undefined,
|
||||
);
|
||||
return preview === undefined ? reduxResolved : preview;
|
||||
}
|
||||
|
||||
@@ -16,7 +16,7 @@
|
||||
* specific language governing permissions and limitations
|
||||
* under the License.
|
||||
*/
|
||||
import { useEffect, useMemo, useState } from 'react';
|
||||
import { useEffect, useMemo, useRef, useState } from 'react';
|
||||
import rison from 'rison';
|
||||
import { useDispatch } from 'react-redux';
|
||||
import { SupersetClient } from '@superset-ui/core';
|
||||
@@ -24,6 +24,7 @@ import { t } from '@apache-superset/core/translation';
|
||||
import { Button, Modal, Select } from '@superset-ui/core/components';
|
||||
import { useToasts } from 'src/components/MessageToasts/withToasts';
|
||||
import { useEffectiveThemeId } from 'src/dashboard/components/ComponentThemeProvider';
|
||||
import { previewThemeStore } from 'src/dashboard/components/ComponentThemeProvider/previewThemeStore';
|
||||
import { setComponentThemeId } from 'src/dashboard/actions/setComponentThemeId';
|
||||
|
||||
interface ThemeOption {
|
||||
@@ -57,17 +58,43 @@ export default function ThemeSelectorModal({
|
||||
const currentThemeId = useEffectiveThemeId(layoutId);
|
||||
|
||||
// Modal-local draft of the selection. Synced from the resolved id when
|
||||
// the modal opens; only committed to Redux on save.
|
||||
// the modal opens; live-previewed via the previewThemeStore as the user
|
||||
// picks options; only committed to Redux on Apply.
|
||||
const [selectedId, setSelectedId] = useState<number | null>(currentThemeId);
|
||||
const [themes, setThemes] = useState<ThemeOption[]>([]);
|
||||
const [loading, setLoading] = useState(false);
|
||||
|
||||
// Keep the draft in sync if the resolved id changes while the modal is
|
||||
// open (e.g. another tab updated the dashboard). Cheap because the
|
||||
// selector returns a primitive.
|
||||
// Snapshot the resolved id at open-time so we can revert correctly when
|
||||
// the user cancels — `currentThemeId` itself is reactive (and would
|
||||
// already reflect the in-flight preview), so we can't use it directly.
|
||||
const initialIdRef = useRef<number | null>(currentThemeId);
|
||||
|
||||
useEffect(() => {
|
||||
if (show) setSelectedId(currentThemeId);
|
||||
}, [show, currentThemeId]);
|
||||
if (show) {
|
||||
initialIdRef.current = currentThemeId;
|
||||
setSelectedId(currentThemeId);
|
||||
}
|
||||
// No `show` cleanup here — the close handlers below clear the preview
|
||||
// explicitly so we don't fight with the Apply path (which keeps the
|
||||
// theme applied).
|
||||
// eslint-disable-next-line react-hooks/exhaustive-deps
|
||||
}, [show]);
|
||||
|
||||
// Push the user's draft selection through the preview store. The
|
||||
// ComponentThemeProvider prefers preview > Redux, so the targeted
|
||||
// component re-renders with the candidate theme as soon as this updates.
|
||||
useEffect(() => {
|
||||
if (!show) return undefined;
|
||||
previewThemeStore.set(layoutId, selectedId);
|
||||
return () => {
|
||||
// Cleanup runs on close + on every selectedId change; the next
|
||||
// effect call re-sets it. On unmount/close we want the preview
|
||||
// gone so the provider re-resolves from Redux. Safe because Apply
|
||||
// commits to Redux *before* hiding the modal, so the post-clear
|
||||
// resolution lands on the saved value, not the original.
|
||||
previewThemeStore.clear(layoutId);
|
||||
};
|
||||
}, [show, layoutId, selectedId]);
|
||||
|
||||
useEffect(() => {
|
||||
if (!show) return;
|
||||
|
||||
Reference in New Issue
Block a user