You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by en...@apache.org on 2026/08/06 16:20:25 UTC
(superset) 16/18: feat(dashboard-v2): sort the header by what each control acts on
This is an automated email from the ASF dual-hosted git repository.
EnxDev pushed a commit to branch dashboard-v2
in repository https://gitbox.apache.org/repos/asf/superset.git
commit c3c1dee74e2c84e1bfd24a0a6e078dd555daf1fc
Author: Enzo Martellucci <en...@gmail.com>
AuthorDate: Thu Aug 6 17:07:45 2026 +0200
feat(dashboard-v2): sort the header by what each control acts on
The bar above the canvas had collected everything: what the dashboard is
called, whether it is saved, and also how the canvas lays blocks out and
when it reloads. The last two are not chrome about the dashboard -- they
act on the blocks in front of you and are reached for while looking at
them -- so they move into the canvas's own top-left corner.
Arrange is a route rather than a second control. How a container lays
out its children is a property of that container, asked with the columns
and the gap it works alongside, and a copy of the switcher here would be
a second thing to keep agreeing with the first: it selects the root, and
the panel brings Properties forward on a selection it did not make.
That would have made the mode unreachable on a blank dashboard, where
the placeholder renders instead of the root and there is nothing to
select -- which is exactly when the mode is worth asking, since whatever
is placed next lands in the one already chosen. The placeholder is now
the dashboard, and selects it.
Refresh is the opposite: named, and honest that it cannot work. There is
no row behind this page and no query to re-run, so it says so rather
than doing nothing quietly. `Inert` is what says that in one place --
disabled, wrapped in a span so the tooltip survives the disabled button.
What is left on the bar is what the dashboard is, in the order it is
read: the name, then who owns it and when it last changed, then the
saving. History moves beside Save because that is what it is a history
of. Templates, History and Save are what an author leaves with and are
sized accordingly.
The canvas padding is two past the token so the corner controls clear
the frame the root draws inside it, written as an offset rather than a
literal so it still moves with the scale.
Co-Authored-By: Claude Opus 5 <no...@anthropic.com>
---
.../pages/DashboardBuilderV2/CanvasControls.tsx | 103 +++++++++++++
.../DashboardBuilderV2/DashboardHeader.test.tsx | 79 ++++++++--
.../pages/DashboardBuilderV2/DashboardHeader.tsx | 162 +++++++++++----------
.../src/pages/DashboardBuilderV2/InertControl.tsx | 99 +++++++++++++
.../src/pages/DashboardBuilderV2/index.test.tsx | 71 ++++++---
.../src/pages/DashboardBuilderV2/index.tsx | 34 ++++-
6 files changed, 439 insertions(+), 109 deletions(-)
diff --git a/superset-frontend/src/pages/DashboardBuilderV2/CanvasControls.tsx b/superset-frontend/src/pages/DashboardBuilderV2/CanvasControls.tsx
new file mode 100644
index 00000000000..d161a53891b
--- /dev/null
+++ b/superset-frontend/src/pages/DashboardBuilderV2/CanvasControls.tsx
@@ -0,0 +1,103 @@
+/**
+ * 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 type { ReactElement } from 'react';
+import { t } from '@apache-superset/core/translation';
+import { useTheme } from '@apache-superset/core/theme';
+import { Button } from '@superset-ui/core/components';
+import { Icons } from '@superset-ui/core/components/Icons';
+import { provider } from 'src/core/dashboard/store';
+import Inert, { compact } from './InertControl';
+
+/**
+ * What acts on the canvas as a whole, in the canvas's own corner.
+ *
+ * These two are not chrome about the dashboard — not what it is called, who
+ * owns it, or whether it is published. They act on the blocks in front of you,
+ * and both are reached for while looking at them, which is why they sit here
+ * rather than on the bar above.
+ *
+ * **Arrange** is a route, not a control. How a container lays out its
+ * children is a property of that container and is asked with the rest of them
+ * — the columns, the gap, the row height it works alongside. That is the
+ * right home for it and also further from hand than something permanently on
+ * screen, so this is the way back to it. A second copy of the switcher would
+ * be a second thing to keep agreeing with the first; selecting the root is
+ * all this does, and the editor panel brings Properties forward on a
+ * selection it did not make itself. It stays live on a page where almost
+ * nothing is, because selecting a node this page already holds in memory
+ * needs no dashboard row.
+ *
+ * **Refresh** is the opposite: named, and honest that it cannot work. There
+ * is no row behind this page and no query to re-run, so it says so rather
+ * than doing nothing quietly.
+ */
+export default function CanvasControls(): ReactElement {
+ const theme = useTheme();
+
+ return (
+ <div
+ data-test="canvas-controls"
+ // In the canvas's own padding, and sized to fit inside it: at the next
+ // step up these are taller than the inset and clip the frame the root
+ // draws, which reads as a collision rather than as a corner.
+ //
+ // Raised because the root is positioned too, and a positioned sibling
+ // earlier in the tree is painted under it — the same tree-order rule
+ // that decides which of two overlapping blocks wins.
+ style={{
+ position: 'absolute',
+ top: theme.sizeUnit,
+ left: theme.sizeUnit,
+ zIndex: 1,
+ display: 'flex',
+ alignItems: 'center',
+ // Two icons of the same size and colour sitting a hair apart read as
+ // one control with two halves. The space is what separates arranging
+ // the canvas from reloading it.
+ gap: theme.sizeUnit * 3,
+ }}
+ >
+ <Button
+ size="small"
+ buttonStyle="link"
+ aria-label={t('Arrange dashboard')}
+ data-test="canvas-arrange"
+ tooltip={t('Arrange dashboard — choose how the canvas lays blocks out')}
+ placement="bottom"
+ style={{ ...compact(theme), paddingInline: theme.sizeUnit }}
+ onClick={() => provider.setSelection(provider.getRoot().id)}
+ >
+ <Icons.LayoutOutlined iconSize="s" />
+ </Button>
+ <Inert
+ label={t('Refresh dashboard')}
+ test="canvas-refresh"
+ buttonStyle="link"
+ // Zeroed because it is not this row's to set. A disabled control is
+ // wrapped in a span so its tooltip survives, and Superset's button
+ // styles give a wrapped button a left margin meant for a row of them
+ // — so this one sat further from its neighbour than the neighbour sat
+ // from anything, and the pair's spacing stopped being the `gap` above.
+ style={{ paddingInline: theme.sizeUnit, marginLeft: 0 }}
+ >
+ <Icons.ReloadOutlined iconSize="s" />
+ </Inert>
+ </div>
+ );
+}
diff --git a/superset-frontend/src/pages/DashboardBuilderV2/DashboardHeader.test.tsx b/superset-frontend/src/pages/DashboardBuilderV2/DashboardHeader.test.tsx
index 4642cf62955..2374cad31d8 100644
--- a/superset-frontend/src/pages/DashboardBuilderV2/DashboardHeader.test.tsx
+++ b/superset-frontend/src/pages/DashboardBuilderV2/DashboardHeader.test.tsx
@@ -23,25 +23,33 @@ import DashboardHeader from './DashboardHeader';
const provider = DashboardProvider.getInstance();
+const ADMIN = { userId: 1, firstName: 'Admin', lastName: 'User' };
+
+/** The header reads the session for who is authoring, so it needs the store. */
+const renderHeader = () =>
+ render(<DashboardHeader />, {
+ useRedux: true,
+ initialState: { user: ADMIN },
+ });
+
beforeEach(() => {
provider.reset();
});
test('the header carries the dashboard-level affordances', () => {
- render(<DashboardHeader />);
+ renderHeader();
expect(screen.getByTestId('header-templates')).toBeInTheDocument();
expect(screen.getByTestId('header-history')).toBeInTheDocument();
expect(screen.getByTestId('header-favorite')).toBeInTheDocument();
expect(screen.getByTestId('header-published')).toHaveTextContent('Draft');
- expect(screen.getByTestId('header-refresh')).toBeInTheDocument();
expect(screen.getByTestId('header-undo')).toBeInTheDocument();
expect(screen.getByTestId('header-redo')).toBeInTheDocument();
expect(screen.getByTestId('header-save')).toBeInTheDocument();
});
test('everything the builder cannot actually do is disabled, not silently dead', () => {
- render(<DashboardHeader />);
+ renderHeader();
// The builder keeps its tree in memory with no dashboard row behind it:
// nothing here can be saved, favourited, published or refreshed, and there
@@ -51,26 +59,69 @@ test('everything the builder cannot actually do is disabled, not silently dead',
'header-templates',
'header-history',
'header-favorite',
- 'header-refresh',
'header-undo',
'header-redo',
'header-save',
].forEach(test => expect(screen.getByTestId(test)).toBeDisabled());
});
-test('the layout switcher is the one live control, and it edits the tree', async () => {
- render(<DashboardHeader />);
- const rootId = provider.getRoot().id;
+test('the record of what was written sits beside writing it', () => {
+ renderHeader();
+
+ const order = [
+ ...screen.getByTestId('dashboard-header').querySelectorAll('[data-test]'),
+ ].map(el => el.getAttribute('data-test'));
+
+ // Saving commits a version; History is the versions already committed.
+ // They are one concern read in one place, so History leaves the far left —
+ // where it sat beside Templates as a thing asked before the work — and
+ // comes to rest immediately before the button that produces what it lists.
+ expect(order.indexOf('header-history')).toBe(
+ order.indexOf('header-save') - 1,
+ );
+});
+
+test('how the dashboard is arranged is not asked in the header', () => {
+ renderHeader();
+
+ // Arranging the canvas is authoring, not chrome. It belongs with the rest
+ // of the root's properties, where the columns and the gap it works with
+ // already live — see Inspector's Arrangement section.
+ expect(screen.queryByTestId('layout-mode-switcher')).not.toBeInTheDocument();
+});
+
+test('what acts on the canvas is not offered from the bar above it', () => {
+ renderHeader();
+
+ // Arranging and refreshing both act on the canvas as a whole, and are both
+ // reached for while looking at it. They sit in its corner — see
+ // CanvasControls. What stays here is what the dashboard is, not what is
+ // being done to the blocks on it.
+ expect(screen.queryByTestId('canvas-arrange')).not.toBeInTheDocument();
+ expect(screen.queryByTestId('header-arrange')).not.toBeInTheDocument();
+ expect(screen.queryByTestId('header-refresh')).not.toBeInTheDocument();
+});
+
+test('the header says who is making the dashboard', () => {
+ renderHeader();
+
+ // The one piece of dashboard metadata this page can state truthfully: a
+ // dashboard being created is being created by whoever is looking at it.
+ expect(screen.getByTestId('header-metadata')).toHaveTextContent('Admin User');
+});
- // Live precisely because its state is in the tree rather than in a row
- // this page does not have.
- await userEvent.click(screen.getByTestId('layout-mode-flex'));
+test('the header does not claim a dashboard with no row behind it was saved', () => {
+ renderHeader();
- expect(provider.getNode(rootId)?.layout?.mode).toBe('flex');
+ // Every other unavailable affordance here says so. A humanized "a day ago"
+ // beside them would be the only thing on the bar inventing a fact.
+ expect(screen.getByTestId('header-metadata')).toHaveTextContent(
+ 'Not saved yet',
+ );
});
test('the dashboard is nameable, and the name is stored on the dashboard', async () => {
- render(<DashboardHeader />);
+ renderHeader();
await userEvent.type(screen.getByTestId('header-title'), 'Vaccine rollout');
await userEvent.tab();
@@ -83,14 +134,14 @@ test('the dashboard is nameable, and the name is stored on the dashboard', async
test('the title shows a rename made anywhere else', () => {
provider.updateProps(provider.getRoot().id, { title: 'From the assistant' });
- render(<DashboardHeader />);
+ renderHeader();
expect(screen.getByTestId('header-title')).toHaveValue('From the assistant');
});
test('emptying the title is not a rename', async () => {
provider.updateProps(provider.getRoot().id, { title: 'Quarterly review' });
- render(<DashboardHeader />);
+ renderHeader();
await userEvent.clear(screen.getByTestId('header-title'));
await userEvent.tab();
diff --git a/superset-frontend/src/pages/DashboardBuilderV2/DashboardHeader.tsx b/superset-frontend/src/pages/DashboardBuilderV2/DashboardHeader.tsx
index 00d04c102a5..27d6464de14 100644
--- a/superset-frontend/src/pages/DashboardBuilderV2/DashboardHeader.tsx
+++ b/superset-frontend/src/pages/DashboardBuilderV2/DashboardHeader.tsx
@@ -17,74 +17,75 @@
* under the License.
*/
import { useEffect, useState } from 'react';
-import type { ReactElement, ReactNode } from 'react';
+import type { ReactElement } from 'react';
+import { useSelector } from 'react-redux';
import { t } from '@apache-superset/core/translation';
import { useTheme } from '@apache-superset/core/theme';
-import {
- Button,
- type ButtonProps,
- Input,
- PublishedLabel,
-} from '@superset-ui/core/components';
+import { Input, PublishedLabel } from '@superset-ui/core/components';
+import MetadataBar, {
+ MetadataType,
+} from '@superset-ui/core/components/MetadataBar';
import { Icons } from '@superset-ui/core/components/Icons';
+import type { BootstrapUser } from 'src/types/bootstrapTypes';
import { provider, useDashboardRevision } from 'src/core/dashboard/store';
-import LayoutModeSwitcher from './LayoutModeSwitcher';
-
-const NOT_AVAILABLE = t('Not available yet');
+import Inert from './InertControl';
/**
- * Header controls, sized down.
+ * Who is making this dashboard, and when it was last written down.
+ *
+ * The two facts a dashboard carries about itself rather than about its
+ * contents, drawn with the same `MetadataBar` the saved dashboard header
+ * uses — so a dashboard being built and one being read state them in the
+ * same shape, with the same icons, in the same place.
*
- * The bar is chrome around the work rather than the work itself, and every
- * pixel it takes is one the canvas does not get. Driven from the theme's own
- * smallest control step rather than a literal, so it tracks the scale the
- * rest of the app is built on instead of drifting from it.
+ * Only one of them can be true here. A dashboard being created is being
+ * created by whoever is looking at it, so the creator is read from the
+ * session rather than invented. There is no row behind this page and nothing
+ * has ever been written, so there is no modified time to humanize — and
+ * "a day ago" beside a Save button that is disabled for having nothing to
+ * save would be the only thing on this bar stating a fact that is not one.
*/
-const compact = (theme: ReturnType<typeof useTheme>) => ({
- height: theme.controlHeightXS,
- paddingInline: theme.sizeUnit * 1.5,
- fontSize: theme.fontSizeSM,
- lineHeight: 1,
-});
-
/**
- * An affordance that is present, named and honest about not working.
- *
- * Most of this header is one. The builder keeps its tree in memory and has
- * no dashboard row behind it: nothing here can be saved, favourited,
- * published or refreshed, and there is no history to step through. Drawing
- * them disabled says which parts of the product this page is still missing;
- * drawing them live and inert would teach something false about all of them.
+ * The signed-in person's name.
*
- * `Button` renders a disabled control inside a span so its tooltip survives —
- * a bare disabled button swallows the pointer events a tooltip listens for,
- * and the explanation would never reach the one control that needs it.
+ * Assembled here rather than through `getUserName`, which reads the
+ * `first_name`/`last_name` an API hands back for an owner. The session user
+ * is the same person in a different shape — `firstName`/`lastName` off the
+ * bootstrap — and passing one to the other returns an empty string rather
+ * than failing, which is how this first went out reading "Not available"
+ * over a perfectly well-known name.
*/
-const Inert = ({
- label,
- test,
- buttonStyle,
- children,
-}: {
- label: string;
- test: string;
- buttonStyle?: ButtonProps['buttonStyle'];
- children: ReactNode;
-}): ReactElement => {
- const theme = useTheme();
+const nameOf = (user: BootstrapUser): string =>
+ [user?.firstName, user?.lastName].filter(Boolean).join(' ') ||
+ user?.username ||
+ '';
+
+const Metadata = (): ReactElement => {
+ const user = useSelector<{ user?: BootstrapUser }, BootstrapUser>(
+ state => state.user,
+ );
+ const author = nameOf(user) || t('Not available');
+ const unsaved = t('Not saved yet');
+
return (
- <Button
- size="small"
- buttonStyle={buttonStyle}
- disabled
- aria-label={label}
- data-test={test}
- tooltip={`${label} — ${NOT_AVAILABLE}`}
- placement="bottom"
- style={compact(theme)}
- >
- {children}
- </Button>
+ <span data-test="header-metadata">
+ <MetadataBar
+ tooltipPlacement="bottom"
+ items={[
+ {
+ type: MetadataType.Editor,
+ createdBy: author,
+ editors: t('None'),
+ createdOn: unsaved,
+ },
+ {
+ type: MetadataType.LastModified,
+ value: unsaved,
+ modifiedBy: author,
+ },
+ ]}
+ />
+ </span>
);
};
@@ -141,10 +142,17 @@ const Title = ({ nodeId, title }: { nodeId: string; title: string }) => {
* The dashboard's header: what this dashboard is, and what can be done to it.
*
* Two kinds of thing share the bar. On the left is the dashboard as the
- * product would know it — where to start from, where it has been, whether it
- * is published. On the right is what an author does to the tree in front of
- * them, and the one live control among them is the layout: it is the only one
- * whose state is in the tree rather than in a row this page does not have.
+ * product would know it — where to start from, where it has been, what it is
+ * called, whether it is published, and whose it is. On the right is what an
+ * author does to the whole of it: step back through what they did, or write
+ * it down.
+ *
+ * How the canvas is arranged is not among them. It reads like chrome and is
+ * not: it is a property of the root node, sitting in the same `layout` the
+ * columns and the gap sit in, and asking for it here put one third of that
+ * one decision on the other side of the screen from the rest. It is asked in
+ * the root's own properties now, where a canvas is selected and arranged in
+ * one place.
*/
export default function DashboardHeader(): ReactElement {
useDashboardRevision();
@@ -164,16 +172,13 @@ export default function DashboardHeader(): ReactElement {
background: theme.colorBgContainer,
}}
>
- {/* Where this dashboard came from and where it has been: one offers a
- starting point to build from, the other the record of what has
- already happened to it. Both are asked before the work rather than
- during it, which is why they lead the bar. */}
- <Inert label={t('Templates')} test="header-templates">
+ {/* Where this dashboard came from: a starting point to build on, asked
+ before the work rather than during it, which is why it leads the
+ bar. History used to sit beside it on that reasoning and has gone to
+ the other end — it is read against saving, not against starting. */}
+ <Inert label={t('Templates')} test="header-templates" reads>
{t('Templates')}
</Inert>
- <Inert label={t('History')} test="header-history">
- {t('History')}
- </Inert>
<Title
nodeId={root.id}
title={typeof root.props?.title === 'string' ? root.props.title : ''}
@@ -186,6 +191,10 @@ export default function DashboardHeader(): ReactElement {
<span data-test="header-published">
<PublishedLabel isPublished={false} />
</span>
+ {/* Beside the status rather than opposite it: whether a dashboard is a
+ draft, whose it is, and when it was last written are one answer to
+ one question — what state is this in — and they are read together. */}
+ <Metadata />
<span
style={{
@@ -195,14 +204,6 @@ export default function DashboardHeader(): ReactElement {
gap: theme.sizeUnit * 2,
}}
>
- <LayoutModeSwitcher nodeId={root.id} />
- <Inert
- label={t('Refresh dashboard')}
- test="header-refresh"
- buttonStyle="link"
- >
- <Icons.ReloadOutlined iconSize="m" />
- </Inert>
{/* Icons, not words, because these two are reached by muscle memory
far more often than they are read. The name stays on them for
anyone not reading with their eyes. */}
@@ -212,7 +213,14 @@ export default function DashboardHeader(): ReactElement {
<Inert label={t('Redo')} test="header-redo">
<Icons.RedoOutlined iconSize="s" />
</Inert>
- <Inert label={t('Save')} test="header-save">
+ {/* Saving commits a version; History is the versions already
+ committed. One concern, read in one place — so the record sits
+ immediately before the button that produces what it lists, rather
+ than at the far side of the bar from it. */}
+ <Inert label={t('History')} test="header-history" reads>
+ {t('History')}
+ </Inert>
+ <Inert label={t('Save')} test="header-save" reads>
{t('Save')}
</Inert>
</span>
diff --git a/superset-frontend/src/pages/DashboardBuilderV2/InertControl.tsx b/superset-frontend/src/pages/DashboardBuilderV2/InertControl.tsx
new file mode 100644
index 00000000000..072372dcfa1
--- /dev/null
+++ b/superset-frontend/src/pages/DashboardBuilderV2/InertControl.tsx
@@ -0,0 +1,99 @@
+/**
+ * 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 type { ReactElement, ReactNode } from 'react';
+import { t } from '@apache-superset/core/translation';
+import { useTheme } from '@apache-superset/core/theme';
+import { Button, type ButtonProps } from '@superset-ui/core/components';
+
+const NOT_AVAILABLE = t('Not available yet');
+
+/**
+ * This prototype's controls, at two sizes.
+ *
+ * Nothing here is drawn at full size: these bars are chrome around the work
+ * rather than the work itself, and every pixel they take is one the canvas
+ * does not get. But the two kinds of control on them are not read the same
+ * way. A word is read, and one squeezed to the smallest step the theme has is
+ * read slowly; an icon is recognised by its shape, and loses nothing there.
+ *
+ * Both are driven off the theme's own control scale rather than literals, so
+ * they track the size the rest of the app is built on instead of drifting.
+ */
+export const named = (theme: ReturnType<typeof useTheme>) => ({
+ height: theme.controlHeightSM,
+ paddingInline: theme.sizeUnit * 2.5,
+ fontSize: theme.fontSize,
+ lineHeight: 1,
+});
+
+export const compact = (theme: ReturnType<typeof useTheme>) => ({
+ height: theme.controlHeightXS,
+ paddingInline: theme.sizeUnit * 1.5,
+ fontSize: theme.fontSizeSM,
+ lineHeight: 1,
+});
+
+/**
+ * An affordance that is present, named and honest about not working.
+ *
+ * Most of this prototype's chrome is one. The builder keeps its tree in
+ * memory and has no dashboard row behind it: nothing can be saved,
+ * favourited, published or refreshed, and there is no history to step
+ * through. Drawing them disabled says which parts of the product this page is
+ * still missing; drawing them live and inert would teach something false
+ * about all of them.
+ *
+ * `Button` renders a disabled control inside a span so its tooltip survives —
+ * a bare disabled button swallows the pointer events a tooltip listens for,
+ * and the explanation would never reach the one control that needs it. That
+ * is the whole reason this is a component rather than a prop spread at each
+ * site, and it is why a second home for it did not get a second copy.
+ */
+export default function Inert({
+ label,
+ test,
+ buttonStyle,
+ /** Whether this one is read as a word rather than recognised as a shape. */
+ reads,
+ style,
+ children,
+}: {
+ label: string;
+ test: string;
+ buttonStyle?: ButtonProps['buttonStyle'];
+ reads?: boolean;
+ style?: ButtonProps['style'];
+ children: ReactNode;
+}): ReactElement {
+ const theme = useTheme();
+ return (
+ <Button
+ size="small"
+ buttonStyle={buttonStyle}
+ disabled
+ aria-label={label}
+ data-test={test}
+ tooltip={`${label} — ${NOT_AVAILABLE}`}
+ placement="bottom"
+ style={{ ...(reads ? named(theme) : compact(theme)), ...style }}
+ >
+ {children}
+ </Button>
+ );
+}
diff --git a/superset-frontend/src/pages/DashboardBuilderV2/index.test.tsx b/superset-frontend/src/pages/DashboardBuilderV2/index.test.tsx
index a08195d16b2..25d03df4200 100644
--- a/superset-frontend/src/pages/DashboardBuilderV2/index.test.tsx
+++ b/superset-frontend/src/pages/DashboardBuilderV2/index.test.tsx
@@ -31,33 +31,70 @@ beforeEach(() => {
provider.reset();
});
-test('a blank dashboard still offers a layout to arrange it in', () => {
- render(<DashboardBuilderV2 />);
+const renderPage = () => render(<DashboardBuilderV2 />, { useRedux: true });
+
+test('a blank dashboard can still be reached, and so can the layout it arranges in', async () => {
+ renderPage();
- // The page a `/dashboard/v2/new/` load lands on has nothing on it yet, and
- // that is exactly when someone reaches for the layout control: whatever is
- // placed next lands in the mode already chosen, rather than being placed
- // and then rearranged.
- expect(screen.getByTestId('layout-mode-switcher')).toBeInTheDocument();
// The canvas is no longer chat-only: a palette sits beside it, so the
// empty state names both ways in.
expect(
screen.getByText('Add a building block, or ask the assistant to start'),
).toBeInTheDocument();
+
+ // A `/dashboard/v2/new/` load lands on nothing, and that is exactly when
+ // someone reaches for the layout control: whatever is placed next lands in
+ // the mode already chosen, rather than being placed and then rearranged.
+ // Arranging is asked in the root's own properties, so the blank canvas has
+ // to be selectable or the mode is unreachable until something is placed.
+ await userEvent.click(screen.getByTestId('empty-canvas'));
+
+ expect(provider.getSelection()).toBe(provider.getRoot().id);
+ expect(screen.getByTestId('layout-mode-switcher')).toBeInTheDocument();
});
-test('the layout control survives the first block being added', () => {
+test('selecting the root offers the layout once blocks have been placed too', async () => {
provider.addBuildingBlock(provider.getRoot().id, 0, { type: 'markdown' });
+ renderPage();
+
+ provider.setSelection(provider.getRoot().id);
+
+ expect(await screen.findByTestId('layout-mode-switcher')).toBeInTheDocument();
+});
+
+test('the canvas carries the route to how it is arranged', async () => {
+ provider.addBuildingBlock(provider.getRoot().id, 0, { type: 'markdown' });
+ renderPage();
+
+ // On the thing it arranges rather than on the bar above it: choosing how
+ // blocks lay out is done while looking at the blocks, and the control it
+ // leads to is one selection away in the root's own properties.
+ await userEvent.click(screen.getByTestId('canvas-arrange'));
+
+ expect(provider.getSelection()).toBe(provider.getRoot().id);
+ expect(screen.getByTestId('layout-mode-switcher')).toBeInTheDocument();
+});
+
+test('the arrange shortcut is offered on a blank canvas too', () => {
+ renderPage();
+
+ // A blank dashboard is exactly when the mode is chosen — whatever is placed
+ // next lands in it.
+ expect(screen.getByTestId('canvas-arrange')).toBeInTheDocument();
+});
- render(<DashboardBuilderV2 />);
+test('refreshing sits with arranging, and is honest about not working', () => {
+ renderPage();
- expect(screen.getAllByTestId('layout-mode-switcher').length).toBeGreaterThan(
- 0,
- );
+ // Both act on the canvas as a whole rather than on anything placed in it,
+ // so they are reached for from the same corner. Refreshing is still the
+ // affordance this builder cannot honour: there is no dashboard row behind
+ // the page and nothing to re-read.
+ expect(screen.getByTestId('canvas-refresh')).toBeDisabled();
});
test('the page is a header, an editor panel and a canvas', () => {
- render(<DashboardBuilderV2 />);
+ renderPage();
expect(screen.getByTestId('dashboard-header')).toBeInTheDocument();
expect(screen.getByTestId('editor-panel')).toBeInTheDocument();
@@ -65,7 +102,7 @@ test('the page is a header, an editor panel and a canvas', () => {
});
test('placing a block from the palette puts it on the dashboard and selects it', async () => {
- render(<DashboardBuilderV2 />);
+ renderPage();
await userEvent.click(screen.getByTestId('palette-markdown'));
@@ -77,7 +114,7 @@ test('placing a block from the palette puts it on the dashboard and selects it',
});
test('a block placed while a container is selected goes inside it', async () => {
- render(<DashboardBuilderV2 />);
+ renderPage();
await userEvent.click(screen.getByTestId('palette-canvas'));
const sectionId = provider.getSelection()!;
@@ -92,7 +129,7 @@ test('a block placed while a container is selected goes inside it', async () =>
});
test('a block placed while a leaf is selected goes beside it, not inside it', async () => {
- render(<DashboardBuilderV2 />);
+ renderPage();
await userEvent.click(screen.getByTestId('palette-markdown'));
const firstId = provider.getSelection()!;
@@ -105,7 +142,7 @@ test('a block placed while a leaf is selected goes beside it, not inside it', as
});
test('clicking the canvas itself clears the selection', async () => {
- render(<DashboardBuilderV2 />);
+ renderPage();
await userEvent.click(screen.getByTestId('palette-markdown'));
expect(provider.getSelection()).toBeDefined();
diff --git a/superset-frontend/src/pages/DashboardBuilderV2/index.tsx b/superset-frontend/src/pages/DashboardBuilderV2/index.tsx
index 90eaa94e38c..6f794484e49 100644
--- a/superset-frontend/src/pages/DashboardBuilderV2/index.tsx
+++ b/superset-frontend/src/pages/DashboardBuilderV2/index.tsx
@@ -27,6 +27,7 @@ import { placeBlock } from 'src/core/dashboard/placement';
import { chat } from 'src/core/chat';
import BuildingBlockView from 'src/core/dashboard/BuildingBlockView';
import { dashboardClientTools } from './clientTools';
+import CanvasControls from './CanvasControls';
import DashboardHeader from './DashboardHeader';
import EditorPanel from './EditorPanel';
@@ -44,7 +45,13 @@ const Canvas = styled.div`
flex: 1;
min-height: 0;
overflow: auto;
- padding: ${theme.paddingLG}px;
+ /* What the canvas's own corner controls are positioned against. */
+ position: relative;
+ /* Two past the token, so the corner controls clear the frame the root
+ draws inside this padding rather than sitting hard against it. Written
+ as an offset from the token rather than as a literal, so it still moves
+ with the scale the rest of the app is built on. */
+ padding: ${theme.paddingLG + 2}px;
`}
`;
@@ -143,13 +150,38 @@ export default function DashboardBuilderV2() {
}
}}
>
+ {/* The canvas's own corner: what acts on the whole of it rather
+ than on anything placed in it, and so belongs to it rather than
+ to the bar above. First in the tree, raised over the root by its
+ own z-index — see CanvasControls. */}
+ <CanvasControls />
{isEmpty ? (
<EmptyCanvasWrapper>
+ {/* The dashboard itself, standing in for a canvas that has
+ nothing on it yet. It selects the root because that is the
+ only thing there is to select here, and because how the
+ canvas is arranged is asked in the root's properties — a
+ blank dashboard is exactly when that is asked, since
+ whatever is placed next lands in the mode already chosen.
+ Without this the mode would be unreachable until something
+ had already been placed and then rearranged. */}
<CanvasPlaceholder
vertical
align="center"
justify="center"
gap="small"
+ // eslint-disable-next-line jsx-a11y/prefer-tag-over-role
+ role="button"
+ tabIndex={0}
+ aria-label={t('Dashboard')}
+ data-test="empty-canvas"
+ onClick={() => provider.setSelection(root.id)}
+ onKeyDown={event => {
+ if (event.key === 'Enter' || event.key === ' ') {
+ event.preventDefault();
+ provider.setSelection(root.id);
+ }
+ }}
>
<Icons.AppstoreOutlined iconSize="xl" />
<Typography.Text type="secondary">