allow renaming tabs by double clicking name (#30465)

* allow user to rename selected tab directly

* fix style

* change behavior to double click and refactor defualt exports

* add unit tests

frontend/src/metabase/core/components/TabButton/TabButton.styled.tsx

@@ -18,6 +18,7 @@ export const TabButtonInputWrapper = styled.span`
 export const TabButtonInputResizer = styled.span`
   visibility: hidden;
   white-space: pre;
+  padding-right: 2px;
 `;
 
 export const TabButtonInput = styled.input<TabButtonProps & { value: string }>`
@@ -26,7 +27,8 @@ export const TabButtonInput = styled.input<TabButtonProps & { value: string }>`
   left: 0;
   padding: 0;
 
-  border: none;
+  border: 1px solid transparent;
+  border-radius: 4px;
   outline: none;
   background-color: transparent;
 
@@ -40,6 +42,15 @@ export const TabButtonInput = styled.input<TabButtonProps & { value: string }>`
     css`
       pointer-events: none;
     `}
+
+  &:hover,
+  :focus {
+    ${props =>
+      (props.isSelected || !props.disabled) &&
+      css`
+        border: 1px solid ${color("border")};
+      `}
+  }
 `;
 
 export const TabButtonRoot = styled.div<TabButtonProps>`

frontend/src/metabase/core/components/TabButton/TabButton.tsx

@@ -22,6 +22,7 @@ import {
   TabContext,
   TabContextType,
 } from "../Tab";
+import { TabButtonMenu } from "./TabButtonMenu";
 import {
   TabButtonInput,
   TabButtonRoot,
@@ -29,7 +30,8 @@ import {
   TabButtonInputWrapper,
   TabButtonInputResizer,
 } from "./TabButton.styled";
-import TabButtonMenu from "./TabButtonMenu";
+
+export const INPUT_WRAPPER_TEST_ID = "tab-button-input-wrapper";
 
 export type TabButtonMenuAction<T> = (
   context: TabContextType,
@@ -46,22 +48,24 @@ export interface TabButtonProps<T> extends HTMLAttributes<HTMLDivElement> {
   value: T;
   showMenu?: boolean;
   menuItems?: TabButtonMenuItem<T>[];
-  onEdit?: ChangeEventHandler<HTMLInputElement>;
-  onFinishEditing?: () => void;
-  isEditing?: boolean;
+  onRename?: ChangeEventHandler<HTMLInputElement>;
+  onFinishRenaming?: () => void;
+  isRenaming?: boolean;
+  onInputDoubleClick?: MouseEventHandler<HTMLSpanElement>;
   disabled?: boolean;
 }
 
-const TabButton = forwardRef(function TabButton<T>(
+const _TabButton = forwardRef(function TabButton<T>(
   {
     value,
     menuItems,
     label,
     onClick,
-    onEdit,
-    onFinishEditing,
+    onRename,
+    onFinishRenaming,
+    onInputDoubleClick,
     disabled = false,
-    isEditing = false,
+    isRenaming = false,
     showMenu: showMenuProp = true,
     ...props
   }: TabButtonProps<T>,
@@ -114,7 +118,10 @@ const TabButton = forwardRef(function TabButton<T>(
       aria-label={label}
       id={getTabId(idPrefix, value)}
     >
-      <TabButtonInputWrapper>
+      <TabButtonInputWrapper
+        onDoubleClick={onInputDoubleClick}
+        data-testid={INPUT_WRAPPER_TEST_ID}
+      >
         <TabButtonInputResizer aria-hidden="true">
           {label}
         </TabButtonInputResizer>
@@ -122,11 +129,11 @@ const TabButton = forwardRef(function TabButton<T>(
           type="text"
           value={label}
           isSelected={isSelected}
-          disabled={!isEditing}
-          onChange={onEdit}
+          disabled={!isRenaming}
+          onChange={onRename}
           onKeyPress={handleInputKeyPress}
           onFocus={e => e.currentTarget.select()}
-          onBlur={onFinishEditing}
+          onBlur={onFinishRenaming}
           aria-labelledby={getTabId(idPrefix, value)}
           id={getTabButtonInputId(idPrefix, value)}
           ref={inputRef}
@@ -162,10 +169,14 @@ const TabButton = forwardRef(function TabButton<T>(
 });
 
 export interface RenameableTabButtonProps<T>
-  extends Omit<TabButtonProps<T>, "onEdit" | "onFinishEditing" | "isEditing"> {
+  extends Omit<
+    TabButtonProps<T>,
+    "onRename" | "onFinishRenaming" | "isRenaming"
+  > {
   onRename: (newLabel: string) => void;
   renameMenuLabel?: string;
   renameMenuIndex?: number;
+  canRename?: boolean;
 }
 
 export function RenameableTabButton<T>({
@@ -174,11 +185,12 @@ export function RenameableTabButton<T>({
   onRename,
   renameMenuLabel = t`Rename`,
   renameMenuIndex = 0,
+  canRename = true,
   ...props
 }: RenameableTabButtonProps<T>) {
   const [label, setLabel] = useState(labelProp);
   const [prevLabel, setPrevLabel] = useState(label);
-  const [isEditing, setIsEditing] = useState(false);
+  const [isRenaming, setIsRenaming] = useState(false);
   const inputRef = useRef<HTMLInputElement>(null);
 
   useEffect(() => {
@@ -186,10 +198,10 @@ export function RenameableTabButton<T>({
   }, [labelProp]);
 
   useEffect(() => {
-    if (isEditing) {
+    if (isRenaming) {
       inputRef.current?.focus();
     }
-  }, [isEditing]);
+  }, [isRenaming]);
 
   const onFinishEditing = () => {
     if (label.length === 0) {
@@ -198,13 +210,13 @@ export function RenameableTabButton<T>({
       setPrevLabel(label);
       onRename(label);
     }
-    setIsEditing(false);
+    setIsRenaming(false);
   };
 
   const renameItem = {
     label: renameMenuLabel,
     action: () => {
-      setIsEditing(true);
+      setIsRenaming(true);
     },
   };
   const menuItems = [
@@ -214,11 +226,12 @@ export function RenameableTabButton<T>({
   ];
 
   return (
-    <TabButton
+    <_TabButton
       label={label}
-      isEditing={isEditing}
-      onEdit={e => setLabel(e.target.value)}
-      onFinishEditing={onFinishEditing}
+      isRenaming={canRename && isRenaming}
+      onRename={e => setLabel(e.target.value)}
+      onFinishRenaming={onFinishEditing}
+      onInputDoubleClick={() => setIsRenaming(canRename)}
       menuItems={menuItems}
       ref={inputRef}
       {...props}
@@ -226,8 +239,7 @@ export function RenameableTabButton<T>({
   );
 }
 
-// eslint-disable-next-line import/no-default-export -- deprecated usage
-export default Object.assign(TabButton, {
+export const TabButton = Object.assign(_TabButton, {
   Root: TabButtonRoot,
   Renameable: RenameableTabButton,
 });

frontend/src/metabase/core/components/TabButton/TabButton.unit.spec.tsx

@@ -3,8 +3,12 @@ import { fireEvent, render, screen } from "@testing-library/react";
 import userEvent from "@testing-library/user-event";
 import { getIcon } from "__support__/ui";
 
-import TabRow from "../TabRow";
-import TabButton, { RenameableTabButtonProps } from "./TabButton";
+import { TabRow } from "../TabRow";
+import {
+  TabButton,
+  RenameableTabButtonProps,
+  INPUT_WRAPPER_TEST_ID,
+} from "./TabButton";
 
 function setup(props?: Partial<RenameableTabButtonProps<string>>) {
   const action = jest.fn();
@@ -71,4 +75,18 @@ describe("TabButton", () => {
     expect(onRename).toHaveBeenCalledWith(newLabel);
     expect(await screen.findByDisplayValue(newLabel)).toBeInTheDocument();
   });
+
+  it("should allow the user to rename via double click", async () => {
+    const { onRename } = setup();
+
+    userEvent.dblClick(screen.getByTestId(INPUT_WRAPPER_TEST_ID));
+
+    const newLabel = "A new label";
+    const inputEl = screen.getByRole("textbox");
+    userEvent.type(inputEl, newLabel);
+    fireEvent.keyPress(inputEl, { key: "Enter", charCode: 13 });
+
+    expect(onRename).toHaveBeenCalledWith(newLabel);
+    expect(await screen.findByDisplayValue(newLabel)).toBeInTheDocument();
+  });
 });

frontend/src/metabase/core/components/TabButton/TabButtonMenu.tsx

 import React, { useContext } from "react";
+
 import { TabContext } from "../Tab/TabContext";
 import { getTabButtonInputId } from "../Tab/utils";
 import { TabButtonMenuAction, TabButtonMenuItem } from "./TabButton";
@@ -10,8 +11,7 @@ interface TabButtonMenuProps<T> {
   closePopover: () => void;
 }
 
-// eslint-disable-next-line import/no-default-export -- deprecated usage
-export default function TabButtonMenu<T>({
+export function TabButtonMenu<T>({
   menuItems,
   value,
   closePopover,

frontend/src/metabase/core/components/TabButton/index.ts

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

frontend/src/metabase/core/components/TabRow/TabRow.stories.tsx

@@ -2,12 +2,13 @@ import React, { useState } from "react";
 import type { ComponentStory } from "@storybook/react";
 import { useArgs } from "@storybook/client-api";
 
-import TabButton, {
+import {
+  TabButton,
   TabButtonMenuItem,
   TabButtonMenuAction,
 } from "../TabButton";
 import TabLink from "../TabLink";
-import TabRow from "./TabRow";
+import { TabRow } from "./TabRow";
 
 // eslint-disable-next-line import/no-default-export -- deprecated usage
 export default {

frontend/src/metabase/core/components/TabRow/TabRow.styled.tsx

@@ -3,7 +3,7 @@ import styled from "@emotion/styled";
 import { alpha, color } from "metabase/lib/colors";
 import BaseTabList from "metabase/core/components/TabList";
 import TabLink from "metabase/core/components/TabLink";
-import TabButton from "metabase/core/components/TabButton";
+import { TabButton } from "metabase/core/components/TabButton";
 import { space } from "metabase/styled-components/theme";
 
 export const TabList = styled(BaseTabList)`

frontend/src/metabase/core/components/TabRow/TabRow.tsx

@@ -57,9 +57,7 @@ function TabRowInner<T>({
 }
 
 const TabRowInnerWithSize = ExplicitSize()(TabRowInner);
-
-// eslint-disable-next-line import/no-default-export -- deprecated usage
-export default function TabRow<T>(props: TabListProps<T>) {
+export function TabRow<T>(props: TabListProps<T>) {
   return <TabRowInnerWithSize {...props} />;
 }
 

frontend/src/metabase/core/components/TabRow/TabRow.unit.spec.tsx

@@ -2,8 +2,8 @@ import React, { useState } from "react";
 import { render, screen } from "@testing-library/react";
 import userEvent from "@testing-library/user-event";
 
-import TabButton from "../TabButton";
-import TabRow from "./TabRow";
+import { TabButton } from "../TabButton";
+import { TabRow } from "./TabRow";
 
 const TestTabRow = () => {
   const [value, setValue] = useState(1);

frontend/src/metabase/core/components/TabRow/index.ts

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

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

 import React from "react";
 import styled from "@emotion/styled";
 
-import BaseTabButton, {
+import {
+  TabButton as BaseTabButton,
   RenameableTabButtonProps,
 } from "metabase/core/components/TabButton";
 import BaseButton from "metabase/core/components/Button";

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

 import React from "react";
 import { t } from "ttag";
 
-import TabRow from "metabase/core/components/TabRow";
+import { TabRow } from "metabase/core/components/TabRow";
 import { SelectedTabId } from "metabase-types/store";
 
 import {
@@ -40,6 +40,7 @@ export function DashboardTabs({ isEditing }: DashboardTabsProps) {
               value={tab.id}
               label={tab.name}
               onRename={name => renameTab(tab.id, name)}
+              canRename={isEditing}
               showMenu={isEditing}
               menuItems={[
                 {

frontend/src/metabase/dashboard/components/DashboardTabs/DashboardTabs.unit.spec.tsx

@@ -9,6 +9,7 @@ import { ORDERS_ID, SAMPLE_DB_ID } from "metabase-types/api/mocks/presets";
 import { INITIAL_DASHBOARD_STATE } from "metabase/dashboard/constants";
 import { getDefaultTab } from "metabase/dashboard/actions";
 
+import { INPUT_WRAPPER_TEST_ID } from "metabase/core/components/TabButton";
 import { DashboardTabs } from "./DashboardTabs";
 
 const TEST_CARD = createMockCard({
@@ -285,6 +286,19 @@ describe("DashboardTabs", () => {
 
         expect(queryTab(newName)).toBeInTheDocument();
       });
+
+      it("should allow renaming via double click", async () => {
+        setup();
+        const newName = "Another cool new name";
+        const inputWrapperEl = screen.getAllByTestId(INPUT_WRAPPER_TEST_ID)[0];
+        userEvent.dblClick(inputWrapperEl);
+
+        const inputEl = screen.getByRole("textbox", { name: "Page 1" });
+        userEvent.type(inputEl, newName);
+        fireEvent.keyPress(inputEl, { key: "Enter", charCode: 13 });
+
+        expect(queryTab(newName)).toBeInTheDocument();
+      });
     });
   });
 });

frontend/src/metabase/models/components/ModelDetailPage/ModelDetailPage.styled.tsx

 import styled from "@emotion/styled";
 
-import BaseTabRow from "metabase/core/components/TabRow";
+import { TabRow as BaseTabRow } from "metabase/core/components/TabRow";
 import BaseTabPanel from "metabase/core/components/TabPanel";
 
 export const TabRow = styled(BaseTabRow)`