Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
68 changes: 34 additions & 34 deletions desktop/src/app/AppShell.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,12 @@ import { Outlet, useLocation } from "@tanstack/react-router";

import { deriveShellRoute } from "@/app/AppShell.helpers";
import { AppShellProvider } from "@/app/AppShellContext";
import {
AppShellOverlays,
type BrowseDialogType,
} from "@/app/AppShellOverlays";
import { AppShellOverlays } from "@/app/AppShellOverlays";
import { AppTopChrome } from "@/app/AppTopChrome";
import { useAppNavigation } from "@/app/navigation/useAppNavigation";
import { useBackForwardControls } from "@/app/navigation/useBackForwardControls";
import { useLiveHomeFeedActions } from "@/app/useLiveHomeFeedActions";
import { useChannelBrowserDialog } from "@/app/useChannelBrowserDialog";
import { useMarkAsReadShortcuts } from "@/app/useMarkAsReadShortcuts";
import { useSettingsShortcuts } from "@/app/useSettingsShortcuts";
import { useAppShellDesktopNotifications } from "@/app/useAppShellDesktopNotifications";
Expand Down Expand Up @@ -111,8 +109,6 @@ export function AppShell() {
null,
);
const [searchFocusRequest, setSearchFocusRequest] = React.useState(0);
const [browseDialogType, setBrowseDialogType] =
React.useState<BrowseDialogType>(null);
const [isCreateChannelOpen, setIsCreateChannelOpen] = React.useState(false);
const [isSendFeedbackOpen, setIsSendFeedbackOpen] = React.useState(false);
const [isHuddleDrawerOpen, setIsHuddleDrawerOpen] = React.useState(false);
Expand Down Expand Up @@ -416,27 +412,22 @@ export function AppShell() {
[unfollowThread, muteThread],
);

const createChannelMutation = useCreateChannelMutation();
const createForumMutation = useCreateChannelMutation();
const createChannelMutation = useCreateChannelMutation(),
createForumMutation = useCreateChannelMutation();
const { applyCanvas, applyAgents } = useApplyTemplate();

const openDmMutation = useOpenDmMutation();
const hideDmMutation = useHideDmMutation();
const handleOpenBrowseChannels = React.useCallback(() => {
setBrowseDialogType("stream");
void refetchChannels();
}, [refetchChannels]);
const {
browseDialogType,
openBrowseChannels: handleOpenBrowseChannels,
onBrowseDialogOpenChange: handleBrowseDialogOpenChange,
getCreateSuccess,
} = useChannelBrowserDialog(() => void refetchChannels());
const handleOpenSearch = React.useCallback(() => {
setSearchFocusRequest((request) => request + 1);
void refetchChannels();
}, [refetchChannels]);

const handleBrowseDialogOpenChange = React.useCallback((open: boolean) => {
if (!open) {
setBrowseDialogType(null);
}
}, []);

const handleBrowseChannelJoin = React.useCallback(
async (channelId: string) => {
await joinChannel(channelId);
Expand All @@ -446,19 +437,22 @@ export function AppShell() {
);

const handleCreateChannel = React.useCallback(
async ({
description,
name,
visibility,
ttlSeconds,
templateId,
}: {
name: string;
description?: string;
visibility: ChannelVisibility;
ttlSeconds?: number;
templateId?: string;
}) => {
async (
{
description,
name,
visibility,
ttlSeconds,
templateId,
}: {
name: string;
description?: string;
visibility: ChannelVisibility;
ttlSeconds?: number;
templateId?: string;
},
onCreated?: (channelId: string) => void,
) => {
const createdChannel = await createChannelMutation.mutateAsync({
name,
description,
Expand All @@ -469,6 +463,7 @@ export function AppShell() {

await applyCanvas(templateId, createdChannel.id, name);
await goChannel(createdChannel.id);
onCreated?.(createdChannel.id);
void applyAgents(templateId, createdChannel.id);
},
[applyAgents, applyCanvas, createChannelMutation, goChannel],
Expand Down Expand Up @@ -516,10 +511,15 @@ export function AppShell() {
if (browseDialogType === "forum") {
await handleCreateForum(input);
} else {
await handleCreateChannel(input);
await handleCreateChannel(input, getCreateSuccess() ?? undefined);
}
},
[browseDialogType, handleCreateChannel, handleCreateForum],
[
browseDialogType,
handleCreateChannel,
handleCreateForum,
getCreateSuccess,
],
);

const handleHideDm = React.useCallback(
Expand Down
39 changes: 39 additions & 0 deletions desktop/src/app/useChannelBrowserDialog.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
import * as React from "react";

import type { BrowseDialogType } from "@/app/AppShellOverlays";

type CreatedCallback = (channelId: string) => void;

export function useChannelBrowserDialog(onOpen: () => void) {
const [browseDialogType, setBrowseDialogType] =
React.useState<BrowseDialogType>(null);
const createSuccessRef = React.useRef<CreatedCallback | null>(null);

const openBrowseChannels = React.useCallback(
(onCreated?: CreatedCallback) => {
createSuccessRef.current = onCreated ?? null;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Guard browse opener arguments before storing callbacks

When openBrowseChannels is used as an existing React click handler by the welcome-channel intro (ChannelPane.tsx lines 445-450), React passes the click event as the first argument. This line stores that MouseEvent in createSuccessRef, so creating a channel from that dialog later reaches onCreated?.(createdChannel.id) and throws because the stored value is not callable, leaving the create flow in an error state after the channel has been created. Please only store function arguments (or keep the public opener zero-argument and add a separate section-specific API).

Useful? React with 👍 / 👎.

setBrowseDialogType("stream");
onOpen();
},
[onOpen],
);

const onBrowseDialogOpenChange = React.useCallback((open: boolean) => {
if (!open) {
createSuccessRef.current = null;
setBrowseDialogType(null);
}
}, []);

const getCreateSuccess = React.useCallback(
() => createSuccessRef.current,
[],
);

return {
browseDialogType,
openBrowseChannels,
onBrowseDialogOpenChange,
getCreateSuccess,
};
}
12 changes: 11 additions & 1 deletion desktop/src/features/sidebar/ui/AppSidebar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ type AppSidebarProps = {
lastMessageAt: string | null | undefined,
) => void;
onMarkAllChannelsRead: () => void;
onBrowseChannels?: () => void;
onBrowseChannels?: (onCreated?: (channelId: string) => void) => void;
onOpenDm: (input: { pubkeys: string[] }) => Promise<void>;
onUpdateCommunity: (
id: string,
Expand Down Expand Up @@ -530,6 +530,13 @@ export function AppSidebar({
openCreateDialog("stream");
}, [onCreateChannelOpenChange, openCreateDialog]);

const handleCreateChannelInSection = React.useCallback(
(sectionId: string) => {
onBrowseChannels?.((channelId) => assignChannel(channelId, sectionId));
},
[assignChannel, onBrowseChannels],
);

return (
<Sidebar
className="!border-r-0"
Expand Down Expand Up @@ -667,6 +674,9 @@ export function AppSidebar({
onAssignChannel={assignChannel}
onUnassignChannel={unassignChannel}
onCreateSectionForChannel={handleCreateSectionForChannel}
onCreateChannel={() =>
handleCreateChannelInSection(section.id)
}
onRenameSection={() => setRenameSectionTarget(section)}
onDeleteSection={() => setDeleteSectionTarget(section)}
onMoveSectionUp={() => moveSectionUp(section.id)}
Expand Down
7 changes: 7 additions & 0 deletions desktop/src/features/sidebar/ui/CustomChannelSection.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -557,6 +557,7 @@ export function CustomChannelSection({
onAssignChannel,
onUnassignChannel,
onCreateSectionForChannel,
onCreateChannel,
onRenameSection,
onDeleteSection,
onMoveSectionUp,
Expand Down Expand Up @@ -595,6 +596,7 @@ export function CustomChannelSection({
onAssignChannel: (channelId: string, sectionId: string) => void;
onUnassignChannel: (channelId: string) => void;
onCreateSectionForChannel: (channelId: string) => void;
onCreateChannel: () => void;
onRenameSection: () => void;
onDeleteSection: () => void;
onMoveSectionUp: () => void;
Expand Down Expand Up @@ -664,6 +666,11 @@ export function CustomChannelSection({
</button>
</SidebarGroupLabel>
<div className="absolute right-1 top-1/2 z-10 flex -translate-y-1/2 items-center gap-0.5">
<SectionQuickAction
label={`Add channel to ${section.name}`}
onClick={onCreateChannel}
testId={`section-actions-${section.id}-quick-create`}
/>
<SectionActionsMenu
sectionLabel={section.name}
testId={`section-actions-${section.id}`}
Expand Down
95 changes: 92 additions & 3 deletions desktop/tests/e2e/channel-browser.spec.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,29 @@
import { expect, test } from "@playwright/test";
import { expect, test, type Page } from "@playwright/test";

import { installMockBridge, openChannelBrowser } from "../helpers/bridge";

test.beforeEach(async ({ page }) => {
await installMockBridge(page);
const MOCK_PUBKEY = "deadbeef".repeat(8);
const CUSTOM_SECTION = { id: "sec-projects", name: "Projects", order: 0 };

async function seedCustomSection(page: Page) {
await page.addInitScript(
({ pubkey, section }) => {
window.localStorage.setItem(
`buzz-channel-sections.v1:${pubkey}`,
JSON.stringify({ version: 1, sections: [section], assignments: {} }),
);
},
{ pubkey: MOCK_PUBKEY, section: CUSTOM_SECTION },
);
}

test.beforeEach(async ({ page }, testInfo) => {
await installMockBridge(
page,
testInfo.title.includes("failed section create")
? { createChannelErrors: ["Create failed"] }
: undefined,
);
});

test("keyboard shortcut opens the channel browser dialog", async ({ page }) => {
Expand Down Expand Up @@ -172,6 +192,75 @@ test("sidebar add-channel button opens the browser", async ({ page }) => {
await expect(page.getByTestId("channel-browser-dialog")).toBeVisible();
});

test("custom section add button creates directly into that section", async ({
page,
}) => {
await seedCustomSection(page);
await page.goto("/");

const addButton = page.getByTestId(
`section-actions-${CUSTOM_SECTION.id}-quick-create`,
);
await expect(addButton).toHaveAccessibleName("Add channel to Projects");
await addButton.click();
await expect(page.getByTestId("channel-browser-dialog")).toBeVisible();

const channelName = `section-created-${Date.now()}`;
await page.getByTestId("channel-browser-search").fill(channelName);
await page.getByTestId("channel-browser-create-row").click();
await page.getByTestId("create-channel-submit").click();

await expect(page.getByTestId("channel-browser-dialog")).not.toBeVisible();
await expect(
page.getByTestId(`section-title-${CUSTOM_SECTION.id}`),
).toBeVisible();
await expect(page.getByTestId(`channel-${channelName}`)).toBeVisible();
await expect(page.getByTestId("stream-list")).not.toContainText(channelName);
});

test("canceling section create does not affect the next global create", async ({
page,
}) => {
await seedCustomSection(page);
await page.goto("/");

await page
.getByTestId(`section-actions-${CUSTOM_SECTION.id}-quick-create`)
.click();
await page.keyboard.press("Escape");
await expect(page.getByTestId("channel-browser-dialog")).not.toBeVisible();

await page.getByTestId("section-actions-channels-quick-create").click();
const channelName = `global-after-cancel-${Date.now()}`;
await page.getByTestId("channel-browser-search").fill(channelName);
await page.getByTestId("channel-browser-create-row").click();
await page.getByTestId("create-channel-submit").click();

await expect(page.getByTestId("stream-list")).toContainText(channelName);
});

test("failed section create retry still assigns to the section", async ({
page,
}) => {
await seedCustomSection(page);
await page.goto("/");

await page
.getByTestId(`section-actions-${CUSTOM_SECTION.id}-quick-create`)
.click();
const channelName = `section-retry-${Date.now()}`;
await page.getByTestId("channel-browser-search").fill(channelName);
await page.getByTestId("channel-browser-create-row").click();
await page.getByTestId("create-channel-submit").click();
await expect(page.getByText("Create failed")).toBeVisible();

await page.getByTestId("create-channel-submit").click();

await expect(page.getByTestId("channel-browser-dialog")).not.toBeVisible();
await expect(page.getByTestId(`channel-${channelName}`)).toBeVisible();
await expect(page.getByTestId("stream-list")).not.toContainText(channelName);
});

test("create affordance is visible on open before typing", async ({ page }) => {
await page.goto("/");

Expand Down