From e36669d9c61573819ca97bc40d5b32a6ffc6416f Mon Sep 17 00:00:00 2001
From: Nemanja Glumac <31325167+nemanjaglumac@users.noreply.github.com>
Date: Tue, 2 Apr 2024 19:42:57 +0200
Subject: [PATCH] SQL in a sidebar - Milestone 2: Visual tweaks (#40694)
* Move all notebook containers styles to one place
* Make NotebookStep wide again
* Tweak sidebar width and breakpoints
* Remove top margin from NotebookSteps
* Stop line wrapping in an ace editor
* Uniform width between step header and step body
* Remove stray styles
* Scroll content independently
* Use named export
* Use Mantine button for "Visualize"
* Convert notebook step width to rem
* Adjust the position of the step close button
* Adjust notebook step max-width to 1200px
* Remove the `cx` where not needed
* Use HTML 5 `aside` element for the sidebar
* Use Mantine responsive styles
* Fix E2E test
---
.../scenarios/question/notebook.cy.spec.js | 2 +-
.../components/notebook/Notebook.styled.tsx | 13 -------
.../components/notebook/Notebook.tsx | 11 +++---
.../NotebookStep/NotebookStep.styled.tsx | 7 ++--
.../NotebookSteps/NotebookSteps.styled.tsx | 5 ---
.../notebook/NotebookSteps/NotebookSteps.tsx | 12 ++-----
.../notebook/NotebookSteps/index.ts | 3 +-
.../NativeQueryPreviewSidebar.module.css | 11 ------
.../NativeQueryPreviewSidebar.tsx | 11 +-----
.../NotebookContainer.module.css | 35 +++++++++++++++++++
.../NotebookContainer/NotebookContainer.tsx | 25 +++++++++----
11 files changed, 66 insertions(+), 69 deletions(-)
delete mode 100644 frontend/src/metabase/query_builder/components/notebook/Notebook.styled.tsx
delete mode 100644 frontend/src/metabase/query_builder/components/notebook/NotebookSteps/NotebookSteps.styled.tsx
delete mode 100644 frontend/src/metabase/query_builder/components/view/NativeQueryPreviewSidebar/NativeQueryPreviewSidebar.module.css
create mode 100644 frontend/src/metabase/query_builder/components/view/View/NotebookContainer/NotebookContainer.module.css
diff --git a/e2e/test/scenarios/question/notebook.cy.spec.js b/e2e/test/scenarios/question/notebook.cy.spec.js
index f80f3d7c354..b33bfcecbe4 100644
--- a/e2e/test/scenarios/question/notebook.cy.spec.js
+++ b/e2e/test/scenarios/question/notebook.cy.spec.js
@@ -773,7 +773,7 @@ describe("scenarios > question > notebook", { tags: "@slow" }, () => {
});
getNotebookStep("summarize").within(() => {
cy.findByTestId("aggregate-step").within(() => {
- moveElement({ name: "Count", vertical: 100, index: 3 });
+ moveElement({ name: "Count", vertical: 100, index: 4 });
});
cy.findByTestId("breakout-step").within(() => {
moveElement({ name: "ID", horizontal: 100, index: 1 });
diff --git a/frontend/src/metabase/query_builder/components/notebook/Notebook.styled.tsx b/frontend/src/metabase/query_builder/components/notebook/Notebook.styled.tsx
deleted file mode 100644
index 2d9e13aff96..00000000000
--- a/frontend/src/metabase/query_builder/components/notebook/Notebook.styled.tsx
+++ /dev/null
@@ -1,13 +0,0 @@
-import styled from "@emotion/styled";
-
-import { breakpointMinSmall } from "metabase/styled-components/theme";
-
-export const NotebookRoot = styled.div`
- position: relative;
- padding: 0 1rem;
- margin-bottom: 2rem;
- flex: 1;
- ${breakpointMinSmall} {
- padding: 0 2rem;
- }
-`;
diff --git a/frontend/src/metabase/query_builder/components/notebook/Notebook.tsx b/frontend/src/metabase/query_builder/components/notebook/Notebook.tsx
index 48e82097567..72a6a93d9e7 100644
--- a/frontend/src/metabase/query_builder/components/notebook/Notebook.tsx
+++ b/frontend/src/metabase/query_builder/components/notebook/Notebook.tsx
@@ -1,10 +1,10 @@
import { t } from "ttag";
import _ from "underscore";
-import Button from "metabase/core/components/Button";
import Questions from "metabase/entities/questions";
import { useDispatch } from "metabase/lib/redux";
import { setUIControls } from "metabase/query_builder/actions";
+import { Box, Button } from "metabase/ui";
import * as Lib from "metabase-lib";
import type Question from "metabase-lib/v1/Question";
import {
@@ -13,8 +13,7 @@ import {
} from "metabase-lib/v1/metadata/utils/saved-questions";
import type { State } from "metabase-types/store";
-import { NotebookRoot } from "./Notebook.styled";
-import NotebookSteps from "./NotebookSteps";
+import { NotebookSteps } from "./NotebookSteps";
interface NotebookOwnProps {
className?: string;
@@ -82,14 +81,14 @@ const Notebook = ({ className, updateQuestion, ...props }: NotebookProps) => {
};
return (
- <NotebookRoot className={className}>
+ <Box pos="relative" p={{ base: "1rem", sm: "2rem" }}>
<NotebookSteps updateQuestion={handleUpdateQuestion} {...props} />
{hasVisualizeButton && isRunnable && (
- <Button medium primary style={{ minWidth: 220 }} onClick={visualize}>
+ <Button variant="filled" style={{ minWidth: 220 }} onClick={visualize}>
{t`Visualize`}
</Button>
)}
- </NotebookRoot>
+ </Box>
);
};
diff --git a/frontend/src/metabase/query_builder/components/notebook/NotebookStep/NotebookStep.styled.tsx b/frontend/src/metabase/query_builder/components/notebook/NotebookStep/NotebookStep.styled.tsx
index ec3433b8ff0..55cbdccb3fe 100644
--- a/frontend/src/metabase/query_builder/components/notebook/NotebookStep/NotebookStep.styled.tsx
+++ b/frontend/src/metabase/query_builder/components/notebook/NotebookStep/NotebookStep.styled.tsx
@@ -24,13 +24,10 @@ export interface StepHeaderProps {
export const StepContent = styled.div`
width: ${getPercentage(11 / 12)};
-
- ${breakpointMinSmall} {
- width: ${getPercentage(8 / 12)};
- }
+ max-width: 75rem;
`;
-export const StepHeader = styled(StepContent)<StepHeaderProps>`
+export const StepHeader = styled(StepContent)`
display: flex;
color: ${props => props.color};
font-weight: bold;
diff --git a/frontend/src/metabase/query_builder/components/notebook/NotebookSteps/NotebookSteps.styled.tsx b/frontend/src/metabase/query_builder/components/notebook/NotebookSteps/NotebookSteps.styled.tsx
deleted file mode 100644
index 92910a82c6e..00000000000
--- a/frontend/src/metabase/query_builder/components/notebook/NotebookSteps/NotebookSteps.styled.tsx
+++ /dev/null
@@ -1,5 +0,0 @@
-import styled from "@emotion/styled";
-
-export const Container = styled.div`
- padding-top: 1.5rem;
-`;
diff --git a/frontend/src/metabase/query_builder/components/notebook/NotebookSteps/NotebookSteps.tsx b/frontend/src/metabase/query_builder/components/notebook/NotebookSteps/NotebookSteps.tsx
index af93c4bc7b6..09a6ffb86ab 100644
--- a/frontend/src/metabase/query_builder/components/notebook/NotebookSteps/NotebookSteps.tsx
+++ b/frontend/src/metabase/query_builder/components/notebook/NotebookSteps/NotebookSteps.tsx
@@ -10,8 +10,6 @@ import NotebookStep from "../NotebookStep";
import { getQuestionSteps } from "../lib/steps";
import type { NotebookStep as INotebookStep, OpenSteps } from "../types";
-import { Container } from "./NotebookSteps.styled";
-
interface NotebookStepsProps {
className?: string;
question: Question;
@@ -35,8 +33,7 @@ function getInitialOpenSteps(question: Question, readOnly: boolean): OpenSteps {
return {};
}
-function NotebookSteps({
- className,
+export function NotebookSteps({
question,
sourceQuestion,
reportTimezone,
@@ -92,7 +89,7 @@ function NotebookSteps({
}
return (
- <Container className={className}>
+ <>
{steps.map((step, index) => {
const isLast = index === steps.length - 1;
const isLastOpened = lastOpenedStep === step.id;
@@ -114,9 +111,6 @@ function NotebookSteps({
/>
);
})}
- </Container>
+ </>
);
}
-
-// eslint-disable-next-line import/no-default-export -- deprecated usage
-export default NotebookSteps;
diff --git a/frontend/src/metabase/query_builder/components/notebook/NotebookSteps/index.ts b/frontend/src/metabase/query_builder/components/notebook/NotebookSteps/index.ts
index 9d6f81d4031..9ec10978086 100644
--- a/frontend/src/metabase/query_builder/components/notebook/NotebookSteps/index.ts
+++ b/frontend/src/metabase/query_builder/components/notebook/NotebookSteps/index.ts
@@ -1,2 +1 @@
-// eslint-disable-next-line import/no-default-export -- deprecated usage
-export { default } from "./NotebookSteps";
+export { NotebookSteps } from "./NotebookSteps";
diff --git a/frontend/src/metabase/query_builder/components/view/NativeQueryPreviewSidebar/NativeQueryPreviewSidebar.module.css b/frontend/src/metabase/query_builder/components/view/NativeQueryPreviewSidebar/NativeQueryPreviewSidebar.module.css
deleted file mode 100644
index 4924724373b..00000000000
--- a/frontend/src/metabase/query_builder/components/view/NativeQueryPreviewSidebar/NativeQueryPreviewSidebar.module.css
+++ /dev/null
@@ -1,11 +0,0 @@
-.container {
- position: absolute;
- inset: 0;
- background-color: white;
-
- @media screen and (--breakpoint-min-md) {
- position: relative;
- flex: 1;
- max-width: 37.5rem;
- }
-}
diff --git a/frontend/src/metabase/query_builder/components/view/NativeQueryPreviewSidebar/NativeQueryPreviewSidebar.tsx b/frontend/src/metabase/query_builder/components/view/NativeQueryPreviewSidebar/NativeQueryPreviewSidebar.tsx
index 92dcb725ff4..eed771fe138 100644
--- a/frontend/src/metabase/query_builder/components/view/NativeQueryPreviewSidebar/NativeQueryPreviewSidebar.tsx
+++ b/frontend/src/metabase/query_builder/components/view/NativeQueryPreviewSidebar/NativeQueryPreviewSidebar.tsx
@@ -1,4 +1,3 @@
-import cx from "classnames";
import { useCallback } from "react";
import AceEditor from "react-ace";
import { t } from "ttag";
@@ -15,7 +14,6 @@ import { getQuestion } from "metabase/query_builder/selectors";
import { Box, Button, Flex, Icon, rem } from "metabase/ui";
import * as Lib from "metabase-lib";
-import SB from "./NativeQueryPreviewSidebar.module.css";
import { createDatasetQuery } from "./utils";
const TITLE = {
@@ -64,13 +62,7 @@ export const NativeQueryPreviewSidebar = (): JSX.Element => {
const borderStyle = `1px solid ${color("border")}`;
return (
- <Flex
- data-testid="native-query-preview-sidebar"
- className={cx(SB.container)}
- role="complementary"
- direction="column"
- style={{ borderLeft: borderStyle }}
- >
+ <Flex direction="column" w="100%" h="100%" bg={color("white")}>
<Box
component="header"
c={color("text-dark")}
@@ -107,7 +99,6 @@ export const NativeQueryPreviewSidebar = (): JSX.Element => {
fontSize={12}
style={{ backgroundColor: color("bg-light") }}
showPrintMargin={false}
- wrapEnabled={true}
/>
</NativeQueryEditorRoot>
)}
diff --git a/frontend/src/metabase/query_builder/components/view/View/NotebookContainer/NotebookContainer.module.css b/frontend/src/metabase/query_builder/components/view/View/NotebookContainer/NotebookContainer.module.css
new file mode 100644
index 00000000000..85f6c3d7c33
--- /dev/null
+++ b/frontend/src/metabase/query_builder/components/view/View/NotebookContainer/NotebookContainer.module.css
@@ -0,0 +1,35 @@
+:root {
+ --notebook-width: 40rem;
+ --sidebar-width: 428px;
+}
+
+.notebookContainer {
+ position: absolute;
+ inset: 0;
+ z-index: 2;
+ overflow-y: hidden;
+}
+
+.main {
+ flex: 1;
+ overflow-y: auto;
+
+ @media screen and (--breakpoint-min-lg) {
+ min-width: var(--notebook-width);
+ }
+}
+
+.sqlSidebar {
+ position: absolute;
+ inset: 0;
+
+ @media screen and (--breakpoint-min-lg) {
+ position: relative;
+ flex: 0;
+ border-left: 1px solid var(--color-border);
+ }
+
+ @media screen and (--breakpoint-min-lg) {
+ flex-basis: var(--sidebar-width);
+ }
+}
diff --git a/frontend/src/metabase/query_builder/components/view/View/NotebookContainer/NotebookContainer.tsx b/frontend/src/metabase/query_builder/components/view/View/NotebookContainer/NotebookContainer.tsx
index aeaf1d12723..2f0c55ffeb4 100644
--- a/frontend/src/metabase/query_builder/components/view/View/NotebookContainer/NotebookContainer.tsx
+++ b/frontend/src/metabase/query_builder/components/view/View/NotebookContainer/NotebookContainer.tsx
@@ -5,7 +5,9 @@ import { useSelector } from "metabase/lib/redux";
import Notebook from "metabase/query_builder/components/notebook/Notebook";
import { NativeQueryPreviewSidebar } from "metabase/query_builder/components/view/NativeQueryPreviewSidebar";
import { getUiControls } from "metabase/query_builder/selectors";
-import { Flex } from "metabase/ui";
+import { Flex, Box } from "metabase/ui";
+
+import NC from "./NotebookContainer.module.css";
// There must exist some transition time, no matter how short,
// because we need to trigger the 'onTransitionEnd' in the component
@@ -39,20 +41,29 @@ export const NotebookContainer = ({
return (
<Flex
+ className={NC.notebookContainer}
bg="white"
- pos="absolute"
- inset={0}
opacity={isOpen ? 1 : 0}
style={{
- zIndex: 2,
- overflowY: "auto",
transform: transformStyle,
transition: `transform ${delayBeforeNotRenderingNotebook}ms, opacity ${delayBeforeNotRenderingNotebook}ms`,
}}
onTransitionEnd={handleTransitionEnd}
>
- {shouldShowNotebook && <Notebook {...props} />}
- {isNativePreviewSidebarOpen && <NativeQueryPreviewSidebar />}
+ {shouldShowNotebook && (
+ <Box className={NC.main}>
+ <Notebook {...props} />
+ </Box>
+ )}
+
+ {isNativePreviewSidebarOpen && (
+ <aside
+ className={NC.sqlSidebar}
+ data-testid="native-query-preview-sidebar"
+ >
+ <NativeQueryPreviewSidebar />
+ </aside>
+ )}
</Flex>
);
};
--
GitLab