Move the "remove" button to the sidebar footer (#38050)

* Convert Sidebar to TSX
* Move remove button to the sidebar footer
* Change styles to styled components
* Fix tests

frontend/src/metabase/dashboard/components/DashboardSidebars.jsx

@@ -4,7 +4,7 @@ import _ from "underscore";
 
 import { SIDEBAR_NAME } from "metabase/dashboard/constants";
 
-import ParameterSidebar from "metabase/parameters/components/ParameterSidebar";
+import { ParameterSidebar } from "metabase/parameters/components/ParameterSidebar";
 import SharingSidebar from "metabase/sharing/components/SharingSidebar";
 import * as MetabaseAnalytics from "metabase/lib/analytics";
 import { ClickBehaviorSidebar } from "./ClickBehaviorSidebar/ClickBehaviorSidebar";

frontend/src/metabase/dashboard/components/Sidebar.jsx

-import PropTypes from "prop-types";
-import { t } from "ttag";
-import Button from "metabase/core/components/Button";
-
-const WIDTH = 384;
-
-const propTypes = {
-  closeIsDisabled: PropTypes.bool,
-  children: PropTypes.node,
-  onClose: PropTypes.func,
-  onCancel: PropTypes.func,
-  "data-testid": PropTypes.string,
-};
-
-export function Sidebar({
-  closeIsDisabled,
-  children,
-  onClose,
-  onCancel,
-  "data-testid": dataTestId,
-}) {
-  return (
-    <aside
-      data-testid={dataTestId}
-      style={{ width: WIDTH, minWidth: WIDTH }}
-      className="flex flex-column border-left bg-white"
-    >
-      <div className="flex flex-column flex-auto overflow-y-auto">
-        {children}
-      </div>
-      {(onClose || onCancel) && (
-        <div
-          className="flex align-center border-top"
-          style={{
-            paddingTop: 12,
-            paddingBottom: 12,
-            paddingRight: 32,
-            paddingLeft: 32,
-          }}
-        >
-          {onCancel && (
-            <Button small borderless onClick={onCancel}>{t`Cancel`}</Button>
-          )}
-          {onClose && (
-            <Button
-              primary
-              small
-              className="ml-auto"
-              onClick={onClose}
-              disabled={closeIsDisabled}
-            >{t`Done`}</Button>
-          )}
-        </div>
-      )}
-    </aside>
-  );
-}
-
-Sidebar.propTypes = propTypes;

frontend/src/metabase/dashboard/components/Sidebar.styled.tsx

+import styled from "@emotion/styled";
+import { color } from "metabase/lib/colors";
+
+export const SidebarAside = styled.aside<{ $width: number }>`
+  display: flex;
+  flex-direction: column;
+  width: ${props => props.$width}px;
+  min-width: ${props => props.$width}px;
+  border-left: 1px solid ${color("border")};
+  background: ${color("bg-white")};
+`;
+
+export const ChildrenContainer = styled.div`
+  display: flex;
+  flex-direction: column;
+  flex: auto;
+  overflow-y: auto;
+`;
+
+export const ButtonContainer = styled.div`
+  display: flex;
+  align-items: center;
+  gap: 20px;
+  padding: 12px 32px;
+  border-top: 1px solid ${color("border")};
+`;

frontend/src/metabase/dashboard/components/Sidebar.tsx

+import { t } from "ttag";
+import type { ReactNode } from "react";
+import ButtonDeprecated from "metabase/core/components/Button";
+import { Button, Icon } from "metabase/ui";
+import {
+  ButtonContainer,
+  ChildrenContainer,
+  SidebarAside,
+} from "./Sidebar.styled";
+
+interface SidebarProps {
+  closeIsDisabled?: boolean;
+  children: ReactNode;
+  onClose?: () => void;
+  onCancel?: () => void;
+  onRemove?: () => void;
+  "data-testid"?: string;
+}
+
+export function Sidebar({
+  closeIsDisabled,
+  children,
+  onClose,
+  onCancel,
+  onRemove,
+  "data-testid": dataTestId,
+}: SidebarProps) {
+  return (
+    <SidebarAside data-testid={dataTestId} $width={384}>
+      <ChildrenContainer>{children}</ChildrenContainer>
+      {(onClose || onCancel || onRemove) && (
+        <ButtonContainer>
+          {onRemove && (
+            <Button
+              leftIcon={<Icon name="trash" />}
+              variant="subtle"
+              color="error"
+              onClick={onRemove}
+              style={{ paddingLeft: 0, paddingRight: 0 }}
+              compact
+              role="button"
+              aria-label={t`Remove`}
+            >{t`Remove`}</Button>
+          )}
+          {onCancel && (
+            <ButtonDeprecated
+              small
+              borderless
+              onClick={onCancel}
+            >{t`Cancel`}</ButtonDeprecated>
+          )}
+          {onClose && (
+            <ButtonDeprecated
+              primary
+              small
+              className="ml-auto"
+              onClick={onClose}
+              disabled={closeIsDisabled}
+            >{t`Done`}</ButtonDeprecated>
+          )}
+        </ButtonContainer>
+      )}
+    </SidebarAside>
+  );
+}

frontend/src/metabase/parameters/components/ParameterSettings/ParameterSettings.styled.tsx

 import styled from "@emotion/styled";
 import { color } from "metabase/lib/colors";
-import IconButtonWrapper from "metabase/components/IconButtonWrapper";
 import ParameterValueWidget from "../ParameterValueWidget";
 
 export const SettingsRoot = styled.div`
@@ -25,12 +24,3 @@ export const SettingValueWidget = styled(ParameterValueWidget)`
   border-radius: 0.5rem;
   background-color: ${color("white")};
 `;
-
-export const SettingRemoveButton = styled(IconButtonWrapper)`
-  color: ${color("text-medium")};
-  font-weight: bold;
-
-  &:hover {
-    color: ${color("error")};
-  }
-`;

frontend/src/metabase/parameters/components/ParameterSettings/ParameterSettings.tsx

@@ -14,7 +14,6 @@ import { isSingleOrMultiSelectable } from "../../utils/parameter-type";
 import ValuesSourceSettings from "../ValuesSourceSettings";
 import {
   SettingLabel,
-  SettingRemoveButton,
   SettingSection,
   SettingsRoot,
   SettingValueWidget,
@@ -34,7 +33,6 @@ export interface ParameterSettingsProps {
   onChangeQueryType: (queryType: ValuesQueryType) => void;
   onChangeSourceType: (sourceType: ValuesSourceType) => void;
   onChangeSourceConfig: (sourceConfig: ValuesSourceConfig) => void;
-  onRemoveParameter: () => void;
 }
 
 const ParameterSettings = ({
@@ -46,7 +44,6 @@ const ParameterSettings = ({
   onChangeQueryType,
   onChangeSourceType,
   onChangeSourceConfig,
-  onRemoveParameter,
 }: ParameterSettingsProps): JSX.Element => {
   const [tempLabelValue, setTempLabelValue] = useState(parameter.name);
 
@@ -123,9 +120,6 @@ const ParameterSettings = ({
           />
         </SettingSection>
       )}
-      <SettingRemoveButton onClick={onRemoveParameter}>
-        {t`Remove`}
-      </SettingRemoveButton>
     </SettingsRoot>
   );
 };
@@ -149,5 +143,4 @@ function getLabelError({
   return null;
 }
 
-// eslint-disable-next-line import/no-default-export -- deprecated usage
-export default ParameterSettings;
+export { ParameterSettings };

frontend/src/metabase/parameters/components/ParameterSettings/ParameterSettings.unit.spec.tsx

@@ -2,7 +2,7 @@ import userEvent from "@testing-library/user-event";
 import { renderWithProviders, screen } from "__support__/ui";
 import type { UiParameter } from "metabase-lib/parameters/types";
 import { createMockUiParameter } from "metabase-lib/parameters/mock";
-import ParameterSettings from "../ParameterSettings";
+import { ParameterSettings } from "../ParameterSettings";
 
 interface SetupOpts {
   parameter?: UiParameter;
@@ -111,7 +111,6 @@ const setup = ({ parameter = createMockUiParameter() }: SetupOpts = {}) => {
       onChangeQueryType={onChangeQueryType}
       onChangeSourceType={jest.fn()}
       onChangeSourceConfig={jest.fn()}
-      onRemoveParameter={jest.fn()}
     />,
   );
 

frontend/src/metabase/parameters/components/ParameterSettings/index.ts

-// eslint-disable-next-line import/no-default-export -- deprecated usage
-export { default } from "./ParameterSettings";
+export { ParameterSettings } from "./ParameterSettings";

frontend/src/metabase/parameters/components/ParameterSidebar/ParameterSidebar.tsx

@@ -11,7 +11,7 @@ import type {
 } from "metabase-types/api";
 import { slugify } from "metabase/lib/formatting";
 import { canUseLinkedFilters } from "../../utils/linked-filters";
-import ParameterSettings from "../ParameterSettings";
+import { ParameterSettings } from "../ParameterSettings";
 import ParameterLinkedFilters from "../ParameterLinkedFilters";
 import { SidebarBody, SidebarHeader } from "./ParameterSidebar.styled";
 
@@ -45,7 +45,7 @@ export interface ParameterSidebarProps {
   onClose: () => void;
 }
 
-const ParameterSidebar = ({
+export const ParameterSidebar = ({
   parameter,
   otherParameters,
   onChangeName,
@@ -124,7 +124,7 @@ const ParameterSidebar = ({
   );
 
   return (
-    <Sidebar onClose={onClose}>
+    <Sidebar onClose={onClose} onRemove={handleRemove}>
       <SidebarHeader>
         <Radio
           value={tab}
@@ -144,7 +144,6 @@ const ParameterSidebar = ({
             onChangeQueryType={handleQueryTypeChange}
             onChangeSourceType={handleSourceTypeChange}
             onChangeSourceConfig={handleSourceConfigChange}
-            onRemoveParameter={handleRemove}
           />
         ) : (
           <ParameterLinkedFilters
@@ -168,6 +167,3 @@ const getTabs = (parameter: Parameter) => {
 
   return tabs;
 };
-
-// eslint-disable-next-line import/no-default-export -- deprecated usage
-export default ParameterSidebar;

frontend/src/metabase/parameters/components/ParameterSidebar/ParameterSidebar.unit.spec.tsx

@@ -3,7 +3,7 @@ import { useState } from "react";
 import { renderWithProviders, screen } from "__support__/ui";
 import type { UiParameter } from "metabase-lib/parameters/types";
 import { createMockUiParameter } from "metabase-lib/parameters/mock";
-import ParameterSidebar from "./ParameterSidebar";
+import { ParameterSidebar } from "./ParameterSidebar";
 
 interface SetupOpts {
   initialParameter: UiParameter;

frontend/src/metabase/parameters/components/ParameterSidebar/index.ts

-// eslint-disable-next-line import/no-default-export -- deprecated usage
-export { default } from "./ParameterSidebar";
+export { ParameterSidebar } from "./ParameterSidebar";