mirror of
https://github.com/block/buzz.git
synced 2026-08-18 06:50:31 +02:00
fix(desktop): refine focused thread dismissal targets (#2644)
This commit is contained in:
@@ -68,6 +68,7 @@ import { useDueReminderBadgeCount } from "@/features/reminders/hooks";
|
||||
import { RemindMeLaterProvider } from "@/features/reminders/ui/RemindMeLaterProvider";
|
||||
import { useReminderNotifications } from "@/features/reminders/useReminderNotifications";
|
||||
import { AppSidebar } from "@/features/sidebar/ui/AppSidebar";
|
||||
import { requestFocusedThreadClose } from "@/features/channels/focusedThreadCloseRequest";
|
||||
import { CommunityRail } from "@/features/sidebar/ui/CommunityRail";
|
||||
import { useChannelMutes } from "@/features/sidebar/lib/useChannelMutes";
|
||||
import { useChannelStars } from "@/features/sidebar/lib/useChannelStars";
|
||||
@@ -844,6 +845,7 @@ export function AppShell() {
|
||||
addCommunityDialog.onOpenChange
|
||||
}
|
||||
onNewMessage={handleOpenNewDm}
|
||||
onBackgroundClick={requestFocusedThreadClose}
|
||||
onCreateChannelOpenChange={setIsCreateChannelOpen}
|
||||
onOpenAddCommunity={addCommunityDialog.openDialog}
|
||||
onSendFeedback={() => setIsSendFeedbackOpen(true)}
|
||||
|
||||
@@ -0,0 +1,21 @@
|
||||
import assert from "node:assert/strict";
|
||||
import test from "node:test";
|
||||
|
||||
import {
|
||||
requestFocusedThreadClose,
|
||||
subscribeToFocusedThreadCloseRequest,
|
||||
} from "./focusedThreadCloseRequest.ts";
|
||||
|
||||
test("focus thread close requests reach active subscribers only", () => {
|
||||
let calls = 0;
|
||||
const unsubscribe = subscribeToFocusedThreadCloseRequest(() => {
|
||||
calls += 1;
|
||||
});
|
||||
|
||||
requestFocusedThreadClose();
|
||||
assert.equal(calls, 1);
|
||||
|
||||
unsubscribe();
|
||||
requestFocusedThreadClose();
|
||||
assert.equal(calls, 1);
|
||||
});
|
||||
@@ -0,0 +1,16 @@
|
||||
const listeners = new Set<() => void>();
|
||||
|
||||
/** Request dismissal of an open focus-mode thread drawer. */
|
||||
export function requestFocusedThreadClose(): void {
|
||||
for (const listener of listeners) {
|
||||
listener();
|
||||
}
|
||||
}
|
||||
|
||||
/** Subscribe the active channel surface to focus-mode dismissal requests. */
|
||||
export function subscribeToFocusedThreadCloseRequest(
|
||||
listener: () => void,
|
||||
): () => void {
|
||||
listeners.add(listener);
|
||||
return () => listeners.delete(listener);
|
||||
}
|
||||
@@ -514,16 +514,16 @@ export const ChannelPane = React.memo(function ChannelPane({
|
||||
const isOverlay = useIsThreadPanelOverlay();
|
||||
const useSplitAuxiliaryPane = !isSinglePanelView && !isOverlay;
|
||||
const threadViewMode = useThreadViewMode();
|
||||
// Focus mode is a wide-viewport-only alternative to the split thread pane:
|
||||
// narrow viewports keep their existing single-panel / floating-overlay
|
||||
// behavior untouched. It applies to the thread panel only — channel
|
||||
// management, agent session and profile panels always use the split pane.
|
||||
// Focus mode only replaces the wide split thread pane; narrow threads and
|
||||
// other auxiliary panels keep their existing presentation.
|
||||
const useFocusThreadDrawer =
|
||||
threadViewMode === "focus" &&
|
||||
useSplitAuxiliaryPane &&
|
||||
(Boolean(threadHeadMessage) || shouldShowThreadSkeleton);
|
||||
const { channelIsCovered, markExitComplete } =
|
||||
useFocusDrawerPresence(useFocusThreadDrawer);
|
||||
const { channelIsCovered, markExitComplete } = useFocusDrawerPresence(
|
||||
useFocusThreadDrawer,
|
||||
onCloseThread,
|
||||
);
|
||||
const { changeThreadViewMode, layoutScrollTargetId, resolveScrollTarget } =
|
||||
useThreadViewModeSwitch({
|
||||
externalScrollTargetId: threadScrollTargetId,
|
||||
|
||||
@@ -5,6 +5,7 @@ import {
|
||||
THREAD_FOCUS_DRAWER_TRAVEL_PX,
|
||||
THREAD_FOCUS_SLIVER_WIDTH_PX,
|
||||
} from "@/features/channels/lib/threadFocusLayout";
|
||||
import { getThreadViewMode } from "@/features/channels/lib/threadViewModePreference";
|
||||
import { cn } from "@/shared/lib/cn";
|
||||
|
||||
type FocusThreadDrawerProps = {
|
||||
@@ -167,7 +168,11 @@ export function FocusThreadDrawer({
|
||||
return () => {
|
||||
const previousFocus = previousFocusRef.current;
|
||||
requestAnimationFrame(() => {
|
||||
previousFocus?.focus({ preventScroll: true });
|
||||
// A real dismissal keeps focus mode selected; a presentation switch
|
||||
// has already selected split mode and owns focus inside the new panel.
|
||||
if (getThreadViewMode() === "focus") {
|
||||
previousFocus?.focus({ preventScroll: true });
|
||||
}
|
||||
});
|
||||
};
|
||||
}, []);
|
||||
|
||||
@@ -0,0 +1,10 @@
|
||||
import assert from "node:assert/strict";
|
||||
import test from "node:test";
|
||||
|
||||
import { shouldRestoreThreadToggleFocus } from "./ThreadViewModeToggle.tsx";
|
||||
|
||||
test("restores toggle focus for keyboard activation, not pointer clicks", () => {
|
||||
assert.equal(shouldRestoreThreadToggleFocus(0), true);
|
||||
assert.equal(shouldRestoreThreadToggleFocus(1), false);
|
||||
assert.equal(shouldRestoreThreadToggleFocus(2), false);
|
||||
});
|
||||
@@ -7,6 +7,11 @@ import {
|
||||
import { Button } from "@/shared/ui/button";
|
||||
import { Tooltip, TooltipContent, TooltipTrigger } from "@/shared/ui/tooltip";
|
||||
|
||||
/** Preserve focus only when activation did not come from a pointer click. */
|
||||
export function shouldRestoreThreadToggleFocus(clickDetail: number): boolean {
|
||||
return clickDetail === 0;
|
||||
}
|
||||
|
||||
/**
|
||||
* Both glyphs depict the layout the button switches *to*, never the current one.
|
||||
*
|
||||
@@ -50,7 +55,7 @@ const THREAD_VIEW_MODE_TOGGLE = {
|
||||
export function ThreadViewModeToggle({
|
||||
onChange,
|
||||
}: {
|
||||
onChange: (mode: ThreadViewMode) => void;
|
||||
onChange: (mode: ThreadViewMode, restoreFocus: boolean) => void;
|
||||
}) {
|
||||
const viewMode = useThreadViewMode();
|
||||
const { icon: Icon, label, target } = THREAD_VIEW_MODE_TOGGLE[viewMode];
|
||||
@@ -62,7 +67,9 @@ export function ThreadViewModeToggle({
|
||||
aria-label={label}
|
||||
className="shrink-0"
|
||||
data-testid="thread-view-mode-toggle"
|
||||
onClick={() => onChange(target)}
|
||||
onClick={(event) =>
|
||||
onChange(target, shouldRestoreThreadToggleFocus(event.detail))
|
||||
}
|
||||
size="icon"
|
||||
type="button"
|
||||
variant="ghost"
|
||||
|
||||
@@ -1,13 +1,20 @@
|
||||
import * as React from "react";
|
||||
|
||||
/** Keeps the covered channel inert until the focus drawer finishes exiting. */
|
||||
export function useFocusDrawerPresence(open: boolean) {
|
||||
import { subscribeToFocusedThreadCloseRequest } from "@/features/channels/focusedThreadCloseRequest";
|
||||
|
||||
/** Keeps the covered channel inert and owns external dismissal while open. */
|
||||
export function useFocusDrawerPresence(open: boolean, onClose: () => void) {
|
||||
const [present, setPresent] = React.useState(false);
|
||||
|
||||
React.useEffect(() => {
|
||||
if (open) setPresent(true);
|
||||
}, [open]);
|
||||
|
||||
React.useEffect(() => {
|
||||
if (!open) return;
|
||||
return subscribeToFocusedThreadCloseRequest(onClose);
|
||||
}, [onClose, open]);
|
||||
|
||||
const markExitComplete = React.useCallback(() => setPresent(false), []);
|
||||
return {
|
||||
channelIsCovered: open || present,
|
||||
|
||||
@@ -48,7 +48,7 @@ export function useThreadViewModeSwitch({
|
||||
>(null);
|
||||
|
||||
const changeThreadViewMode = React.useCallback(
|
||||
(mode: ThreadViewMode) => {
|
||||
(mode: ThreadViewMode, restoreFocus: boolean) => {
|
||||
const body = document.querySelector<HTMLElement>(
|
||||
'[data-testid="message-thread-body"]',
|
||||
);
|
||||
@@ -61,7 +61,9 @@ export function useThreadViewModeSwitch({
|
||||
requestAnimationFrame(() => {
|
||||
document
|
||||
.querySelector<HTMLElement>(
|
||||
'[data-testid="thread-view-mode-toggle"]',
|
||||
restoreFocus
|
||||
? '[data-testid="thread-view-mode-toggle"]'
|
||||
: '[data-testid="message-thread-body"]',
|
||||
)
|
||||
?.focus({ preventScroll: true });
|
||||
});
|
||||
|
||||
@@ -526,6 +526,7 @@ export function MessageThreadPanel({
|
||||
data-buzz-conversation-scroll
|
||||
data-testid="message-thread-body"
|
||||
onScroll={onScroll}
|
||||
tabIndex={-1}
|
||||
ref={threadBodyRef}
|
||||
>
|
||||
<div
|
||||
|
||||
@@ -0,0 +1,9 @@
|
||||
import assert from "node:assert/strict";
|
||||
import test from "node:test";
|
||||
|
||||
import { isSidebarBackgroundTarget } from "./sidebarBackgroundTarget.ts";
|
||||
|
||||
test("non-DOM event targets are not sidebar background", () => {
|
||||
assert.equal(isSidebarBackgroundTarget(null), false);
|
||||
assert.equal(isSidebarBackgroundTarget({}), false);
|
||||
});
|
||||
@@ -0,0 +1,12 @@
|
||||
const SIDEBAR_BACKGROUND_ATTRIBUTE = "data-sidebar-background";
|
||||
|
||||
/** Whether a sidebar click landed directly on an opted-in blank surface. */
|
||||
export function isSidebarBackgroundTarget(target: EventTarget | null): boolean {
|
||||
const element =
|
||||
typeof Element !== "undefined" && target instanceof Element
|
||||
? target
|
||||
: typeof Node !== "undefined" && target instanceof Node
|
||||
? target.parentElement
|
||||
: null;
|
||||
return element?.hasAttribute(SIDEBAR_BACKGROUND_ATTRIBUTE) ?? false;
|
||||
}
|
||||
@@ -21,6 +21,7 @@ import {
|
||||
} from "@/features/sidebar/lib/channelSortPreference";
|
||||
import { useChannelSortPreference } from "@/features/sidebar/lib/useChannelSortPreference";
|
||||
import { useSidebarScrollLock } from "@/features/sidebar/lib/useSidebarScrollLock";
|
||||
import { isSidebarBackgroundTarget } from "@/features/sidebar/lib/sidebarBackgroundTarget";
|
||||
import { useUnreadOverflow } from "@/features/sidebar/lib/useUnreadOverflow";
|
||||
import {
|
||||
CreateSectionDialog,
|
||||
@@ -162,6 +163,7 @@ type AppSidebarProps = {
|
||||
selfUserStatus?: UserStatus;
|
||||
isPresencePending?: boolean;
|
||||
onNewMessage: () => void;
|
||||
onBackgroundClick?: () => void;
|
||||
isCreateChannelOpen?: boolean;
|
||||
onCreateChannelOpenChange?: (open: boolean) => void;
|
||||
mutedChannelIds?: ReadonlySet<string>;
|
||||
@@ -179,6 +181,7 @@ export function AppSidebar({
|
||||
currentPubkey,
|
||||
fallbackDisplayName,
|
||||
homeBadgeCount,
|
||||
onBackgroundClick,
|
||||
isAddCommunityOpen,
|
||||
isLoading,
|
||||
isCreatingChannel,
|
||||
@@ -550,10 +553,16 @@ export function AppSidebar({
|
||||
className="!border-r-0"
|
||||
collapsible="offcanvas"
|
||||
data-testid="app-sidebar"
|
||||
onClick={(event) => {
|
||||
if (isSidebarBackgroundTarget(event.target)) {
|
||||
onBackgroundClick?.();
|
||||
}
|
||||
}}
|
||||
variant="sidebar"
|
||||
>
|
||||
<div
|
||||
className="relative flex min-h-0 flex-1 flex-col overflow-hidden"
|
||||
data-sidebar-background
|
||||
data-testid="app-sidebar-scroll-anchor"
|
||||
>
|
||||
<AppSidebarPinnedHeader
|
||||
@@ -572,6 +581,7 @@ export function AppSidebar({
|
||||
|
||||
<div
|
||||
className="relative flex min-h-0 flex-1 flex-col"
|
||||
data-sidebar-background
|
||||
data-testid="sidebar-channel-content"
|
||||
>
|
||||
{unreadAboveCount > 0 ? (
|
||||
@@ -585,10 +595,12 @@ export function AppSidebar({
|
||||
|
||||
<SidebarContent
|
||||
className="buzz-sidebar-scrollbar overscroll-none"
|
||||
data-sidebar-background
|
||||
ref={scrollRef}
|
||||
>
|
||||
<div
|
||||
className="flex w-full flex-col gap-2 px-[3px]"
|
||||
data-sidebar-background
|
||||
data-testid="sidebar-scroll-content"
|
||||
>
|
||||
<AppSidebarPrimaryMenu
|
||||
|
||||
@@ -423,7 +423,6 @@ const SidebarRail = React.forwardRef<
|
||||
(
|
||||
{
|
||||
className,
|
||||
onClick,
|
||||
onPointerCancel,
|
||||
onPointerDown,
|
||||
onPointerMove,
|
||||
@@ -438,7 +437,6 @@ const SidebarRail = React.forwardRef<
|
||||
isRailDisabled,
|
||||
sidebarWidth,
|
||||
state,
|
||||
toggleSidebar,
|
||||
} = useSidebar();
|
||||
const resizeStateRef = React.useRef<{
|
||||
currentWidth: number;
|
||||
@@ -451,8 +449,6 @@ const SidebarRail = React.forwardRef<
|
||||
startWidth: number;
|
||||
startX: number;
|
||||
} | null>(null);
|
||||
const suppressClickRef = React.useRef(false);
|
||||
|
||||
const finishResize = React.useCallback(
|
||||
(event: React.PointerEvent<HTMLButtonElement>) => {
|
||||
const resizeState = resizeStateRef.current;
|
||||
@@ -468,13 +464,6 @@ const SidebarRail = React.forwardRef<
|
||||
document.body.style.userSelect = resizeState.previousUserSelect;
|
||||
setIsResizing(false);
|
||||
resizeStateRef.current = null;
|
||||
|
||||
if (resizeState.hasDragged) {
|
||||
suppressClickRef.current = true;
|
||||
window.requestAnimationFrame(() => {
|
||||
suppressClickRef.current = false;
|
||||
});
|
||||
}
|
||||
},
|
||||
[setIsResizing],
|
||||
);
|
||||
@@ -483,25 +472,9 @@ const SidebarRail = React.forwardRef<
|
||||
<button
|
||||
ref={ref}
|
||||
data-sidebar="rail"
|
||||
aria-label="Resize or toggle sidebar"
|
||||
aria-label="Resize sidebar"
|
||||
tabIndex={-1}
|
||||
disabled={isRailDisabled}
|
||||
onClick={(event) => {
|
||||
if (isRailDisabled) {
|
||||
return;
|
||||
}
|
||||
|
||||
if (suppressClickRef.current) {
|
||||
event.preventDefault();
|
||||
event.stopPropagation();
|
||||
return;
|
||||
}
|
||||
|
||||
onClick?.(event);
|
||||
if (!event.defaultPrevented) {
|
||||
toggleSidebar();
|
||||
}
|
||||
}}
|
||||
disabled={isRailDisabled || state !== "expanded"}
|
||||
onPointerCancel={(event) => {
|
||||
onPointerCancel?.(event);
|
||||
finishResize(event);
|
||||
@@ -572,16 +545,12 @@ const SidebarRail = React.forwardRef<
|
||||
onPointerUp?.(event);
|
||||
finishResize(event);
|
||||
}}
|
||||
title="Drag to resize or click to toggle sidebar"
|
||||
title="Drag to resize sidebar"
|
||||
className={cn(
|
||||
"absolute inset-y-0 z-20 hidden w-4 -translate-x-1/2 transition-all ease-linear group-data-[side=left]:-right-4 group-data-[side=right]:left-0 sm:flex",
|
||||
"cursor-col-resize",
|
||||
"after:absolute after:bottom-0 after:left-1/2 after:top-6 after:z-10 after:w-px after:-translate-x-1/2 after:bg-transparent after:content-['']",
|
||||
"[[data-state=collapsed]_&]:cursor-pointer",
|
||||
"group-data-[collapsible=offcanvas]:translate-x-0 group-data-[collapsible=offcanvas]:hover:bg-sidebar",
|
||||
"[[data-side=left][data-collapsible=offcanvas]_&]:-right-2",
|
||||
"[[data-side=right][data-collapsible=offcanvas]_&]:-left-2",
|
||||
"disabled:pointer-events-none disabled:cursor-default",
|
||||
"disabled:pointer-events-none disabled:hidden",
|
||||
className,
|
||||
)}
|
||||
{...props}
|
||||
|
||||
@@ -328,6 +328,16 @@ test("aligns the sidebar search with the channel title outside the Buzz theme",
|
||||
expect(Math.abs(searchCenter - channelTitleCenter)).toBeLessThanOrEqual(2);
|
||||
});
|
||||
|
||||
test("sidebar rail resizes without toggling the sidebar", async ({ page }) => {
|
||||
await page.goto("/");
|
||||
const rail = page.getByRole("button", { name: "Resize sidebar" });
|
||||
await rail.click();
|
||||
await expect(page.getByTestId("app-sidebar")).toBeVisible();
|
||||
|
||||
await page.getByRole("button", { name: "Toggle Sidebar" }).click();
|
||||
await expect(rail).toBeHidden();
|
||||
});
|
||||
|
||||
test("resizes, persists, and snaps to the default sidebar width", async ({
|
||||
page,
|
||||
}) => {
|
||||
|
||||
@@ -187,13 +187,20 @@ test("focus and split preserve reading context and interaction ownership", async
|
||||
});
|
||||
const anchorId = await topVisibleMessageId(body);
|
||||
|
||||
await page
|
||||
.getByRole("button", { name: "Show thread beside channel" })
|
||||
.click();
|
||||
const focusModeToggle = page.getByRole("button", {
|
||||
name: "Show thread beside channel",
|
||||
});
|
||||
await focusModeToggle.hover();
|
||||
await expect(
|
||||
page.getByRole("tooltip", { name: "Show thread beside channel" }),
|
||||
).toBeVisible();
|
||||
await focusModeToggle.click();
|
||||
await expect(drawer).toHaveCount(0);
|
||||
await expect(channel).not.toHaveAttribute("inert", "");
|
||||
await expectChannelHeaderUnobscured(page);
|
||||
await expect(page.getByTestId("thread-view-mode-toggle")).toBeFocused();
|
||||
await expect(page.getByRole("tooltip")).toHaveCount(0);
|
||||
await expect(page.getByTestId("message-thread-body")).toBeFocused();
|
||||
await expect(summary).not.toBeFocused();
|
||||
await expect(
|
||||
body.locator(`[data-message-id="${anchorId}"]`),
|
||||
).toBeInViewport();
|
||||
@@ -201,7 +208,15 @@ test("focus and split preserve reading context and interaction ownership", async
|
||||
body.locator(`[data-message-id="${anchorId}"]`),
|
||||
).not.toHaveAttribute("data-highlighted", "true");
|
||||
|
||||
await page.getByRole("button", { name: "Expand thread" }).click();
|
||||
// Sidebar background dismissal belongs to the overlay presentation only.
|
||||
await page
|
||||
.getByTestId("app-sidebar-scroll-anchor")
|
||||
.evaluate((element) => (element as HTMLElement).click());
|
||||
await expect(page.getByTestId("message-thread-panel")).toBeVisible();
|
||||
|
||||
const splitModeToggle = page.getByRole("button", { name: "Expand thread" });
|
||||
await splitModeToggle.focus();
|
||||
await splitModeToggle.press("Enter");
|
||||
await expect(drawer).toBeVisible();
|
||||
await expect(channel).toHaveAttribute("inert", "");
|
||||
await expect(page.getByTestId("thread-view-mode-toggle")).toBeFocused();
|
||||
@@ -225,6 +240,19 @@ test("focus and split preserve reading context and interaction ownership", async
|
||||
await expect(page.getByTestId("focus-thread-drawer-overlay")).toHaveCount(0);
|
||||
await expect(channel).not.toHaveAttribute("inert", "");
|
||||
|
||||
await summary.click();
|
||||
await expect(drawer).toBeVisible();
|
||||
const profileCard = page.getByTestId("sidebar-profile-card");
|
||||
await profileCard.click({ position: { x: 8, y: 8 } });
|
||||
await expect(page.getByTestId("profile-popover")).toBeVisible();
|
||||
await expect(drawer).toBeVisible();
|
||||
await profileCard.click({ position: { x: 8, y: 8 } });
|
||||
await expect(page.getByTestId("profile-popover")).toHaveCount(0);
|
||||
await page
|
||||
.getByTestId("app-sidebar-scroll-anchor")
|
||||
.evaluate((element) => (element as HTMLElement).click());
|
||||
await expect(page.getByTestId("focus-thread-drawer-overlay")).toHaveCount(0);
|
||||
|
||||
await summary.click();
|
||||
await expect(drawer).toBeVisible();
|
||||
await page.getByTestId("focus-thread-drawer-scrim").click({
|
||||
|
||||
Reference in New Issue
Block a user