mirror of
https://github.com/bulwarkmail/webmail.git
synced 2026-10-31 00:12:39 +00:00
fix: open the tag picker from the labels hover quick-action
The Labels quick-action on an email row called `onSetTag(null)`, which clears every tag on the message rather than offering one to apply. On an untagged message that writes nothing, so the button appeared dead: no picker, no feedback. Assigning a tag from the opened email worked, since the reading pane renders a real picker. Clicking the row's tag icon now opens the same `TagPicker` used by the context menu and the reading pane, wired to the existing `onSetTag` handler, so a tag can be applied or removed without opening the message. The picker is portalled to the body: every row sits inside a `transform`ed virtual item, which would otherwise make a `fixed` child scroll with the list and clip it at the scroller's edge. It flips above the row near the viewport bottom, and dismisses on outside click, Escape, scroll or resize. The hover toolbar stays visible while the picker is open, since the row is no longer hovered once the pointer moves onto it. Covered by integration/tests/14-hover-tag.spec.ts, which checks the picker opens, the keyword reaches the server under `$label:`, a second click removes it, and the click does not fall through and open the message. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
b526eb1405
commit
e8aeca9602
@@ -1,5 +1,7 @@
|
||||
"use client";
|
||||
|
||||
import { useCallback, useEffect, useRef, useState } from "react";
|
||||
import { createPortal } from "react-dom";
|
||||
import { Email } from "@/lib/jmap/types";
|
||||
import { useSettingsStore } from "@/stores/settings-store";
|
||||
import type { HoverAction } from "@/stores/settings-store";
|
||||
@@ -7,6 +9,8 @@ import { cn } from "@/lib/utils";
|
||||
import { Trash2, Star, Mail, MailOpen, Archive, Tag, ShieldAlert, ShieldCheck } from "lucide-react";
|
||||
import { useTranslations } from "next-intl";
|
||||
import { useIsMobile } from "@/hooks/use-media-query";
|
||||
import { TagPicker } from "@/components/email/tag-picker";
|
||||
import { getEmailTagIds } from "@/lib/thread-utils";
|
||||
|
||||
interface EmailHoverActionsProps {
|
||||
email: Email;
|
||||
@@ -69,6 +73,10 @@ const CORNER_CLASSES = {
|
||||
'bottom-left': 'bottom-1 left-1',
|
||||
} as const;
|
||||
|
||||
/** Roughly the picker's height; used to flip it above the row near the viewport bottom. */
|
||||
const TAG_PICKER_MAX_HEIGHT = 320;
|
||||
const TAG_PICKER_WIDTH = 224;
|
||||
|
||||
export function EmailHoverActions({
|
||||
email,
|
||||
backgroundClassName = "bg-muted",
|
||||
@@ -88,8 +96,43 @@ export function EmailHoverActions({
|
||||
const t = useTranslations("settings.email_behavior.hover_actions");
|
||||
const isMobile = useIsMobile();
|
||||
|
||||
// The tag quick-action opens the same picker the context menu and the reading
|
||||
// pane use. It is portalled to the body because every row sits inside a
|
||||
// `transform`ed virtual item, which would make a `fixed` child scroll with the
|
||||
// list and clip it at the scroller's edge.
|
||||
const [tagPickerPos, setTagPickerPos] = useState<{ top: number; left: number } | null>(null);
|
||||
const pickerRef = useRef<HTMLDivElement | null>(null);
|
||||
const tagButtonRef = useRef<HTMLElement | null>(null);
|
||||
const closeTagPicker = useCallback(() => setTagPickerPos(null), []);
|
||||
|
||||
useEffect(() => {
|
||||
if (!tagPickerPos) return;
|
||||
const onPointerDown = (event: PointerEvent) => {
|
||||
const target = event.target as Node;
|
||||
// The anchor is left to its own click handler, which toggles the picker
|
||||
// shut - closing here first would let that click reopen it.
|
||||
if (tagButtonRef.current?.contains(target)) return;
|
||||
if (!pickerRef.current?.contains(target)) closeTagPicker();
|
||||
};
|
||||
const onKeyDown = (event: KeyboardEvent) => {
|
||||
if (event.key === "Escape") closeTagPicker();
|
||||
};
|
||||
// The anchor moves with the list, so scrolling dismisses rather than chases it.
|
||||
document.addEventListener("pointerdown", onPointerDown, true);
|
||||
document.addEventListener("keydown", onKeyDown);
|
||||
window.addEventListener("scroll", closeTagPicker, true);
|
||||
window.addEventListener("resize", closeTagPicker);
|
||||
return () => {
|
||||
document.removeEventListener("pointerdown", onPointerDown, true);
|
||||
document.removeEventListener("keydown", onKeyDown);
|
||||
window.removeEventListener("scroll", closeTagPicker, true);
|
||||
window.removeEventListener("resize", closeTagPicker);
|
||||
};
|
||||
}, [tagPickerPos, closeTagPicker]);
|
||||
|
||||
const isUnread = !email.keywords?.$seen;
|
||||
const isStarred = email.keywords?.$flagged;
|
||||
const currentTagIds = getEmailTagIds(email.keywords);
|
||||
const hoverBackgroundClassName = backgroundClassName;
|
||||
|
||||
if (isMobile) return null;
|
||||
@@ -111,9 +154,28 @@ export function EmailHoverActions({
|
||||
case "archive":
|
||||
onArchive?.();
|
||||
break;
|
||||
case "tag":
|
||||
onSetTag?.(null);
|
||||
case "tag": {
|
||||
// Previously this cleared every tag on the message, which is a no-op on
|
||||
// an untagged one - hence "clicking it does nothing". Open the picker.
|
||||
if (!onSetTag) break;
|
||||
tagButtonRef.current = e.currentTarget as HTMLElement;
|
||||
const rect = e.currentTarget.getBoundingClientRect();
|
||||
setTagPickerPos((open) =>
|
||||
open
|
||||
? null
|
||||
: {
|
||||
top:
|
||||
rect.bottom + TAG_PICKER_MAX_HEIGHT > window.innerHeight
|
||||
? Math.max(8, rect.top - TAG_PICKER_MAX_HEIGHT - 4)
|
||||
: rect.bottom + 4,
|
||||
left: Math.max(
|
||||
8,
|
||||
Math.min(rect.left, window.innerWidth - TAG_PICKER_WIDTH - 8),
|
||||
),
|
||||
},
|
||||
);
|
||||
break;
|
||||
}
|
||||
case "spam":
|
||||
if (isInJunk) onUndoSpam?.();
|
||||
else onMarkAsSpam?.();
|
||||
@@ -144,8 +206,13 @@ export function EmailHoverActions({
|
||||
return (
|
||||
<button
|
||||
key={actionId}
|
||||
type="button"
|
||||
data-testid={`hover-action-${actionId}`}
|
||||
onClick={(e) => handleAction(e, actionId)}
|
||||
onDoubleClick={(e) => { e.stopPropagation(); e.preventDefault(); }}
|
||||
title={title}
|
||||
aria-haspopup={actionId === "tag" ? "menu" : undefined}
|
||||
aria-expanded={actionId === "tag" ? tagPickerPos !== null : undefined}
|
||||
className={cn(
|
||||
"p-1.5 rounded-md transition-colors duration-100 text-muted-foreground hover:bg-black/5 dark:hover:bg-white/10",
|
||||
className,
|
||||
@@ -161,11 +228,36 @@ export function EmailHoverActions({
|
||||
);
|
||||
});
|
||||
|
||||
const tagPicker =
|
||||
tagPickerPos && onSetTag
|
||||
? createPortal(
|
||||
<div
|
||||
ref={pickerRef}
|
||||
data-testid="hover-tag-picker"
|
||||
className="fixed z-50 w-56 max-w-[18rem] max-h-[20rem] overflow-y-auto rounded-lg border border-border bg-popover py-1 shadow-lg"
|
||||
style={{ top: tagPickerPos.top, left: tagPickerPos.left }}
|
||||
onClick={(e) => e.stopPropagation()}
|
||||
onContextMenu={(e) => e.stopPropagation()}
|
||||
>
|
||||
<TagPicker
|
||||
selectedIds={currentTagIds}
|
||||
onToggle={(tagId) => onSetTag(tagId)}
|
||||
/>
|
||||
</div>,
|
||||
document.body,
|
||||
)
|
||||
: null;
|
||||
|
||||
// While the picker is open the row is no longer hovered, so the toolbar has to
|
||||
// stay mounted and visible or the picker would close under its own anchor.
|
||||
const visibilityClass = tagPickerPos ? "flex" : "hidden group-hover:flex";
|
||||
|
||||
if (hoverActionsMode === 'floating') {
|
||||
return (
|
||||
<div
|
||||
className={cn(
|
||||
"absolute z-10 hidden group-hover:flex items-center",
|
||||
"absolute z-10 items-center",
|
||||
visibilityClass,
|
||||
CORNER_CLASSES[hoverActionsCorner],
|
||||
)}
|
||||
>
|
||||
@@ -175,13 +267,14 @@ export function EmailHoverActions({
|
||||
{actionButtons}
|
||||
</div>
|
||||
</div>
|
||||
{tagPicker}
|
||||
</div>
|
||||
);
|
||||
}
|
||||
|
||||
return (
|
||||
<div
|
||||
className="absolute end-0 top-0 bottom-0 z-10 hidden group-hover:flex items-center"
|
||||
className={cn("absolute end-0 top-0 bottom-0 z-10 items-center", visibilityClass)}
|
||||
>
|
||||
<div
|
||||
className={cn(
|
||||
@@ -198,6 +291,7 @@ export function EmailHoverActions({
|
||||
{actionButtons}
|
||||
</div>
|
||||
</div>
|
||||
{tagPicker}
|
||||
</div>
|
||||
);
|
||||
}
|
||||
|
||||
@@ -0,0 +1,119 @@
|
||||
import { test, expect } from '@playwright/test';
|
||||
import { ACCOUNTS } from './helpers/config';
|
||||
import { sendMail } from './helpers/smtp';
|
||||
import { JmapClient } from './helpers/jmap';
|
||||
import { login, seedSettings, emailItem, expectEmailVisible } from './helpers/app';
|
||||
|
||||
/**
|
||||
* The Labels quick-action on a list row (Settings -> Appearance & layout ->
|
||||
* Hover actions). It used to call `onSetTag(null)`, which clears every tag on
|
||||
* the message - a no-op on an untagged one, so clicking it appeared to do
|
||||
* nothing. It now opens the same tag picker the context menu and the reading
|
||||
* pane use.
|
||||
*/
|
||||
const alice = ACCOUNTS.alice;
|
||||
let seq = 0;
|
||||
const subj = (l: string) => `IT ${l} ${Date.now()}-${seq++}`;
|
||||
const send = (subject: string) =>
|
||||
sendMail({ from: alice.email, authPass: alice.password, to: alice.email, subject, body: 'x' });
|
||||
|
||||
/** The row's tag button, made visible by hovering the row. */
|
||||
async function openHoverTagPicker(page: import('@playwright/test').Page, subject: string) {
|
||||
const row = emailItem(page, subject).first();
|
||||
await row.scrollIntoViewIfNeeded();
|
||||
await row.hover();
|
||||
const button = row.locator('[data-testid="hover-action-tag"]');
|
||||
await expect(button).toBeVisible();
|
||||
await button.click();
|
||||
return page.locator('[data-testid="hover-tag-picker"]');
|
||||
}
|
||||
|
||||
test.describe('Labels hover quick-action', () => {
|
||||
let jmap: JmapClient;
|
||||
|
||||
test.beforeEach(async ({ page }) => {
|
||||
jmap = await JmapClient.connect(alice.email, alice.password);
|
||||
await jmap.reset();
|
||||
// Only the tag quick-action, so the button position is unambiguous.
|
||||
await seedSettings(page, { hoverActions: ['tag'] });
|
||||
});
|
||||
|
||||
test('opens the tag picker and applies the tag to the message', async ({ page }) => {
|
||||
const s = subj('hover-tag');
|
||||
await send(s);
|
||||
await jmap.waitForEmail(s);
|
||||
|
||||
await login(page, alice);
|
||||
await expectEmailVisible(page, s);
|
||||
|
||||
const picker = await openHoverTagPicker(page, s);
|
||||
await expect(picker).toBeVisible();
|
||||
|
||||
const red = picker.getByRole('menuitemcheckbox', { name: 'Red' });
|
||||
await expect(red).toHaveAttribute('aria-checked', 'false');
|
||||
await red.click();
|
||||
|
||||
// The tag reaches the server under the current $label: prefix.
|
||||
await expect(async () => {
|
||||
const mail = await jmap.findEmailBySubject(s);
|
||||
expect(mail?.keywords?.['$label:red']).toBe(true);
|
||||
}).toPass({ timeout: 15000 });
|
||||
|
||||
// ...and the picker reflects it without being reopened.
|
||||
await expect(red).toHaveAttribute('aria-checked', 'true');
|
||||
});
|
||||
|
||||
test('clicking the same tag again removes it', async ({ page }) => {
|
||||
const s = subj('hover-untag');
|
||||
await send(s);
|
||||
await jmap.waitForEmail(s);
|
||||
|
||||
await login(page, alice);
|
||||
await expectEmailVisible(page, s);
|
||||
|
||||
const picker = await openHoverTagPicker(page, s);
|
||||
const red = picker.getByRole('menuitemcheckbox', { name: 'Red' });
|
||||
|
||||
await red.click();
|
||||
await expect(red).toHaveAttribute('aria-checked', 'true');
|
||||
|
||||
await red.click();
|
||||
await expect(red).toHaveAttribute('aria-checked', 'false');
|
||||
|
||||
await expect(async () => {
|
||||
const mail = await jmap.findEmailBySubject(s);
|
||||
expect(mail?.keywords?.['$label:red']).toBeFalsy();
|
||||
}).toPass({ timeout: 15000 });
|
||||
});
|
||||
|
||||
test('stays open when the pointer leaves the row, and closes on Escape', async ({ page }) => {
|
||||
const s = subj('hover-tag-open');
|
||||
await send(s);
|
||||
await jmap.waitForEmail(s);
|
||||
|
||||
await login(page, alice);
|
||||
await expectEmailVisible(page, s);
|
||||
|
||||
const picker = await openHoverTagPicker(page, s);
|
||||
// Moving off the row un-hovers it; the toolbar (and so the picker's anchor)
|
||||
// must not disappear underneath the open picker.
|
||||
await page.mouse.move(5, 5);
|
||||
await expect(picker).toBeVisible();
|
||||
|
||||
await page.keyboard.press('Escape');
|
||||
await expect(picker).toHaveCount(0);
|
||||
});
|
||||
|
||||
test('clicking the tag button does not open the email', async ({ page }) => {
|
||||
const s = subj('hover-tag-noopen');
|
||||
await send(s);
|
||||
await jmap.waitForEmail(s);
|
||||
|
||||
await login(page, alice);
|
||||
await expectEmailVisible(page, s);
|
||||
|
||||
await openHoverTagPicker(page, s);
|
||||
// The reading pane stays closed - the click must not fall through to the row.
|
||||
await expect(page.locator('[data-tour="email-viewer"]')).toHaveCount(0);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user