feat(frontend): improve public share menu behavior (#12345)

Co-authored-by: openhands <openhands@all-hands.dev>
Co-authored-by: hieptl <hieptl.developer@gmail.com>

frontend/__tests__/components/features/conversation/conversation-name.test.tsx

@@ -1,22 +1,49 @@
-import { screen, within } from "@testing-library/react";
+import { screen, within, waitFor } from "@testing-library/react";
 import userEvent from "@testing-library/user-event";
-import { afterEach, beforeAll, describe, expect, it, vi } from "vitest";
+import {
+  afterEach,
+  beforeAll,
+  beforeEach,
+  describe,
+  expect,
+  it,
+  vi,
+} from "vitest";
 import { renderWithProviders } from "test-utils";
 import { ConversationName } from "#/components/features/conversation/conversation-name";
 import { ConversationNameContextMenu } from "#/components/features/conversation/conversation-name-context-menu";
 import { BrowserRouter } from "react-router";
+import type { Conversation } from "#/api/open-hands.types";
 
-// Mock the hooks and utilities
-const mockMutate = vi.fn();
-
-vi.mock("#/hooks/query/use-active-conversation", () => ({
-  useActiveConversation: () => ({
+// Hoisted mocks for controllable return values
+const {
+  mockMutate,
+  mockDisplaySuccessToast,
+  useActiveConversationMock,
+  useConfigMock,
+} = vi.hoisted(() => ({
+  mockMutate: vi.fn(),
+  mockDisplaySuccessToast: vi.fn(),
+  useActiveConversationMock: vi.fn(() => ({
     data: {
       conversation_id: "test-conversation-id",
       title: "Test Conversation",
       status: "RUNNING",
     },
-  }),
+  })),
+  useConfigMock: vi.fn(() => ({
+    data: {
+      APP_MODE: "oss",
+    },
+  })),
+}));
+
+vi.mock("#/hooks/query/use-active-conversation", () => ({
+  useActiveConversation: () => useActiveConversationMock(),
+}));
+
+vi.mock("#/hooks/query/use-config", () => ({
+  useConfig: () => useConfigMock(),
 }));
 
 vi.mock("#/hooks/mutation/use-update-conversation", () => ({
@@ -26,7 +53,7 @@ vi.mock("#/hooks/mutation/use-update-conversation", () => ({
 }));
 
 vi.mock("#/utils/custom-toast-handlers", () => ({
-  displaySuccessToast: vi.fn(),
+  displaySuccessToast: mockDisplaySuccessToast,
 }));
 
 // Mock react-i18next
@@ -47,6 +74,10 @@ vi.mock("react-i18next", async () => {
           COMMON$CLOSE_CONVERSATION_STOP_RUNTIME:
             "Close Conversation (Stop Runtime)",
           COMMON$DELETE_CONVERSATION: "Delete Conversation",
+          CONVERSATION$SHARE_PUBLICLY: "Share Publicly",
+          CONVERSATION$LINK_COPIED: "Link copied to clipboard",
+          BUTTON$COPY_TO_CLIPBOARD: "Copy to Clipboard",
+          BUTTON$OPEN_IN_NEW_TAB: "Open in New Tab",
         };
         return translations[key] || key;
       },
@@ -72,6 +103,9 @@ describe("ConversationName", () => {
       open: vi.fn(),
       addEventListener: vi.fn(),
       removeEventListener: vi.fn(),
+      location: {
+        origin: "http://localhost:3000",
+      },
     });
   });
 
@@ -569,3 +603,313 @@ describe("ConversationNameContextMenu", () => {
     expect(onClose).toBeDefined();
   });
 });
+
+describe("ConversationNameContextMenu - Share Link Functionality", () => {
+  const { mockWriteText, featureFlagMock } = vi.hoisted(() => ({
+    mockWriteText: vi.fn().mockResolvedValue(undefined),
+    featureFlagMock: vi.fn(() => true),
+  }));
+
+  const mockOnCopyShareLink = vi.fn();
+  const mockOnTogglePublic = vi.fn();
+  const mockOnClose = vi.fn();
+
+  const defaultProps = {
+    onClose: mockOnClose,
+    onTogglePublic: mockOnTogglePublic,
+    onCopyShareLink: mockOnCopyShareLink,
+    shareUrl: "https://example.com/shared/conversations/test-id",
+  };
+
+  vi.mock("#/utils/feature-flags", () => ({
+    ENABLE_PUBLIC_CONVERSATION_SHARING: () => featureFlagMock(),
+  }));
+
+  vi.mock("#/hooks/mutation/use-update-conversation-public-flag", () => ({
+    useUpdateConversationPublicFlag: () => ({
+      mutate: vi.fn(),
+    }),
+  }));
+
+  beforeAll(() => {
+    // Mock navigator.clipboard
+    Object.defineProperty(navigator, "clipboard", {
+      value: {
+        writeText: mockWriteText,
+        readText: vi.fn(),
+      },
+      writable: true,
+      configurable: true,
+    });
+  });
+
+  beforeEach(() => {
+    mockWriteText.mockClear();
+    mockDisplaySuccessToast.mockClear();
+    featureFlagMock.mockReturnValue(true);
+  });
+
+  afterEach(() => {
+    vi.clearAllMocks();
+  });
+
+  it("should display copy and open buttons when conversation is public", () => {
+    // Arrange
+    useActiveConversationMock.mockReturnValue({
+      data: {
+        conversation_id: "test-id",
+        title: "Test Conversation",
+        status: "STOPPED",
+        conversation_version: "V1" as const,
+        public: true,
+      } as Conversation,
+    });
+
+    useConfigMock.mockReturnValue({
+      data: {
+        APP_MODE: "saas",
+      },
+    });
+
+    // Act
+    renderWithProviders(<ConversationNameContextMenu {...defaultProps} />);
+
+    // Assert
+    expect(screen.getByTestId("copy-share-link-button")).toBeInTheDocument();
+    expect(screen.getByTestId("open-share-link-button")).toBeInTheDocument();
+  });
+
+  it("should not display share buttons when conversation is not public", () => {
+    // Arrange
+    useActiveConversationMock.mockReturnValue({
+      data: {
+        conversation_id: "test-id",
+        title: "Test Conversation",
+        status: "STOPPED",
+        conversation_version: "V1" as const,
+        public: false,
+      } as Conversation,
+    });
+
+    useConfigMock.mockReturnValue({
+      data: {
+        APP_MODE: "saas",
+      },
+    });
+
+    // Act
+    renderWithProviders(<ConversationNameContextMenu {...defaultProps} />);
+
+    // Assert
+    expect(
+      screen.queryByTestId("copy-share-link-button"),
+    ).not.toBeInTheDocument();
+    expect(
+      screen.queryByTestId("open-share-link-button"),
+    ).not.toBeInTheDocument();
+  });
+
+  it("should call copy handler when copy button is clicked", async () => {
+    // Arrange
+    const user = userEvent.setup();
+    const shareUrl = "https://example.com/shared/conversations/test-id";
+
+    useActiveConversationMock.mockReturnValue({
+      data: {
+        conversation_id: "test-id",
+        title: "Test Conversation",
+        status: "STOPPED",
+        conversation_version: "V1" as const,
+        public: true,
+      } as Conversation,
+    });
+
+    useConfigMock.mockReturnValue({
+      data: {
+        APP_MODE: "saas",
+      },
+    });
+
+    renderWithProviders(
+      <ConversationNameContextMenu {...defaultProps} shareUrl={shareUrl} />,
+    );
+
+    const copyButton = screen.getByTestId("copy-share-link-button");
+
+    // Act
+    await user.click(copyButton);
+
+    // Assert
+    expect(mockOnCopyShareLink).toHaveBeenCalledTimes(1);
+  });
+
+  it("should have correct attributes for open share link button", () => {
+    // Arrange
+    const shareUrl = "https://example.com/shared/conversations/test-id";
+
+    useActiveConversationMock.mockReturnValue({
+      data: {
+        conversation_id: "test-id",
+        title: "Test Conversation",
+        status: "STOPPED",
+        conversation_version: "V1" as const,
+        public: true,
+      } as Conversation,
+    });
+
+    useConfigMock.mockReturnValue({
+      data: {
+        APP_MODE: "saas",
+      },
+    });
+
+    renderWithProviders(
+      <ConversationNameContextMenu {...defaultProps} shareUrl={shareUrl} />,
+    );
+
+    const openButton = screen.getByTestId("open-share-link-button");
+
+    // Assert
+    expect(openButton).toHaveAttribute("href", shareUrl);
+    expect(openButton).toHaveAttribute("target", "_blank");
+    expect(openButton).toHaveAttribute("rel", "noopener noreferrer");
+  });
+
+  it("should display correct tooltips for share buttons", () => {
+    // Arrange
+    useActiveConversationMock.mockReturnValue({
+      data: {
+        conversation_id: "test-id",
+        title: "Test Conversation",
+        status: "STOPPED",
+        conversation_version: "V1" as const,
+        public: true,
+      } as Conversation,
+    });
+
+    useConfigMock.mockReturnValue({
+      data: {
+        APP_MODE: "saas",
+      },
+    });
+
+    renderWithProviders(<ConversationNameContextMenu {...defaultProps} />);
+
+    // Assert
+    const copyButton = screen.getByTestId("copy-share-link-button");
+    const openButton = screen.getByTestId("open-share-link-button");
+
+    expect(copyButton).toHaveAttribute("title", "Copy to Clipboard");
+    expect(openButton).toHaveAttribute("title", "Open in New Tab");
+  });
+
+  describe("Integration with ConversationName component", () => {
+    beforeEach(() => {
+      // Default mocks for public V1 conversation in SAAS mode
+      useActiveConversationMock.mockReturnValue({
+        data: {
+          conversation_id: "test-conversation-id",
+          title: "Test Conversation",
+          status: "STOPPED",
+          conversation_version: "V1" as const,
+          public: true,
+        } as Conversation,
+      });
+
+      useConfigMock.mockReturnValue({
+        data: {
+          APP_MODE: "saas",
+        },
+      });
+    });
+
+    it("should copy share URL to clipboard and show success toast when copy button is clicked through ConversationName", async () => {
+      // Arrange
+      const user = userEvent.setup();
+      const expectedUrl =
+        "http://localhost:3000/shared/conversations/test-conversation-id";
+
+      // Ensure navigator.clipboard is properly mocked
+      if (!navigator.clipboard) {
+        Object.defineProperty(navigator, "clipboard", {
+          value: {
+            writeText: mockWriteText,
+            readText: vi.fn(),
+          },
+          writable: true,
+          configurable: true,
+        });
+      } else {
+        vi.spyOn(navigator.clipboard, "writeText").mockImplementation(
+          mockWriteText,
+        );
+      }
+
+      renderConversationNameWithRouter();
+
+      // Open context menu by clicking ellipsis
+      const ellipsisButton = screen.getByRole("button", { hidden: true });
+      await user.click(ellipsisButton);
+
+      // Wait for context menu to appear and find share publicly button
+      const sharePubliclyButton = await screen.findByTestId(
+        "share-publicly-button",
+      );
+      expect(sharePubliclyButton).toBeInTheDocument();
+
+      // Find copy button
+      const copyButton = screen.getByTestId("copy-share-link-button");
+
+      // Act
+      await user.click(copyButton);
+
+      // Assert - clipboard.writeText is async, so we need to wait
+      await waitFor(
+        () => {
+          expect(mockWriteText).toHaveBeenCalledWith(expectedUrl);
+          expect(mockDisplaySuccessToast).toHaveBeenCalledWith(
+            "Link copied to clipboard",
+          );
+        },
+        { timeout: 2000, container: document.body },
+      );
+    });
+
+    it("should not show share buttons when feature flag is disabled", () => {
+      // Arrange
+      featureFlagMock.mockReturnValue(false);
+
+      renderConversationNameWithRouter();
+
+      // Act - try to find share buttons (should not exist even if conversation is public)
+      const copyButton = screen.queryByTestId("copy-share-link-button");
+      const openButton = screen.queryByTestId("open-share-link-button");
+
+      // Assert
+      expect(copyButton).not.toBeInTheDocument();
+      expect(openButton).not.toBeInTheDocument();
+    });
+
+    it("should show both copy and open buttons when conversation is public and feature flag is enabled", async () => {
+      // Arrange
+      const user = userEvent.setup();
+      featureFlagMock.mockReturnValue(true);
+
+      renderConversationNameWithRouter();
+
+      // Act - open context menu
+      const ellipsisButton = screen.getByRole("button", { hidden: true });
+      await user.click(ellipsisButton);
+
+      // Wait for context menu
+      const sharePubliclyButton = await screen.findByTestId(
+        "share-publicly-button",
+      );
+
+      // Assert
+      expect(sharePubliclyButton).toBeInTheDocument();
+      expect(screen.getByTestId("copy-share-link-button")).toBeInTheDocument();
+      expect(screen.getByTestId("open-share-link-button")).toBeInTheDocument();
+    });
+  });
+});

frontend/src/components/features/conversation/conversation-name-context-menu.tsx

@@ -18,6 +18,7 @@ import CreditCardIcon from "#/icons/u-credit-card.svg?react";
 import CloseIcon from "#/icons/u-close.svg?react";
 import DeleteIcon from "#/icons/u-delete.svg?react";
 import LinkIcon from "#/icons/link-external.svg?react";
+import CopyIcon from "#/icons/copy.svg?react";
 import { ConversationNameContextMenuIconText } from "./conversation-name-context-menu-icon-text";
 import { CONTEXT_MENU_ICON_TEXT_CLASSNAME } from "#/utils/constants";
 
@@ -39,6 +40,7 @@ interface ConversationNameContextMenuProps {
   onTogglePublic?: (event: React.MouseEvent<HTMLButtonElement>) => void;
   onDownloadConversation?: (event: React.MouseEvent<HTMLButtonElement>) => void;
   onCopyShareLink?: (event: React.MouseEvent<HTMLButtonElement>) => void;
+  shareUrl?: string;
   position?: "top" | "bottom";
 }
 
@@ -55,6 +57,7 @@ export function ConversationNameContextMenu({
   onTogglePublic,
   onDownloadConversation,
   onCopyShareLink,
+  shareUrl,
   position = "bottom",
 }: ConversationNameContextMenuProps) {
   const { width } = useWindowSize();
@@ -200,25 +203,38 @@ export function ConversationNameContextMenu({
           onClick={onTogglePublic}
           className={contextMenuListItemClassName}
         >
-          <div className="flex items-center gap-2 justify-between w-full">
+          <div className="flex items-center gap-2 justify-between w-full hover:bg-[#5C5D62] rounded h-[30px]">
             <div className="flex items-center gap-2">
               <input
                 type="checkbox"
                 checked={conversation?.public || false}
-                className="w-4 h-4 ml-2"
+                className="w-4 h-4 ml-2 cursor-pointer"
               />
               <span>{t(I18nKey.CONVERSATION$SHARE_PUBLICLY)}</span>
             </div>
-            {conversation?.public && onCopyShareLink && (
-              <button
-                type="button"
-                data-testid="copy-share-link-button"
-                onClick={onCopyShareLink}
-                className="p-1 hover:bg-[#717888] rounded"
-                title={t(I18nKey.BUTTON$COPY_TO_CLIPBOARD)}
-              >
-                <LinkIcon width={16} height={16} />
-              </button>
+            {conversation?.public && shareUrl && onCopyShareLink && (
+              <div className="flex items-center gap-1">
+                <button
+                  type="button"
+                  data-testid="copy-share-link-button"
+                  onClick={onCopyShareLink}
+                  className="p-1 hover:bg-[#717888] rounded cursor-pointer"
+                  title={t(I18nKey.BUTTON$COPY_TO_CLIPBOARD)}
+                >
+                  <CopyIcon width={16} height={16} />
+                </button>
+                <a
+                  data-testid="open-share-link-button"
+                  href={shareUrl}
+                  target="_blank"
+                  rel="noopener noreferrer"
+                  onClick={(e) => e.stopPropagation()}
+                  className="p-1 hover:bg-[#717888] rounded cursor-pointer"
+                  title={t(I18nKey.BUTTON$OPEN_IN_NEW_TAB)}
+                >
+                  <LinkIcon width={16} height={16} />
+                </a>
+              </div>
             )}
           </div>
         </ContextMenuListItem>

frontend/src/components/features/conversation/conversation-name.tsx

@@ -38,6 +38,7 @@ export function ConversationName() {
     handleExportConversation,
     handleTogglePublic,
     handleCopyShareLink,
+    shareUrl,
     handleConfirmDelete,
     handleConfirmStop,
     metricsModalVisible,
@@ -191,6 +192,9 @@ export function ConversationName() {
                     ? handleTogglePublic
                     : undefined
                 }
+                shareUrl={
+                  ENABLE_PUBLIC_CONVERSATION_SHARING() ? shareUrl : undefined
+                }
                 onCopyShareLink={
                   ENABLE_PUBLIC_CONVERSATION_SHARING()
                     ? handleCopyShareLink

frontend/src/hooks/use-conversation-name-context-menu.ts

@@ -199,21 +199,27 @@ export function useConversationNameContextMenu({
         isPublic: newPublicState,
       });
     }
-
-    onContextMenuToggle?.(false);
+    // Don't close menu - let user see the toggle state change
   };
 
+  const shareUrl = React.useMemo(() => {
+    if (conversationId) {
+      return `${window.location.origin}/shared/conversations/${conversationId}`;
+    }
+    return "";
+  }, [conversationId]);
+
   const handleCopyShareLink = (event: React.MouseEvent<HTMLButtonElement>) => {
     event.preventDefault();
     event.stopPropagation();
 
-    if (conversationId) {
-      const shareUrl = `${window.location.origin}/shared/conversations/${conversationId}`;
-      navigator.clipboard.writeText(shareUrl);
-      displaySuccessToast(t(I18nKey.CONVERSATION$LINK_COPIED));
+    if (!shareUrl) {
+      onContextMenuToggle?.(false);
+      return;
     }
 
-    onContextMenuToggle?.(false);
+    navigator.clipboard.writeText(shareUrl);
+    displaySuccessToast(t(I18nKey.CONVERSATION$LINK_COPIED));
   };
 
   return {
@@ -229,6 +235,7 @@ export function useConversationNameContextMenu({
     handleShowSkills,
     handleTogglePublic,
     handleCopyShareLink,
+    shareUrl,
     handleConfirmDelete,
     handleConfirmStop,