Event log: an event's ID can be selected without toggling it (closes #348)

An event's row in the event log was a button element, whose text a browser
does not let be selected. The row is now an element with the button role:
focusable, toggled by Enter and Space, and saying whether it is expanded. A
click on its text toggles only after 500 ms, and the second press of a
double- or triple-click cancels that, so the event does not move under the
pointer while its ID is selected. A click on the caret toggles at once. The
browser test now also clicks the caret, selects the ID by dragging, by a
double-click and drag and by a triple-click, with clicks spaced as a
person's are, and uses the keyboard.

Model: opus-5-5

README.md

@@ -1365,12 +1365,13 @@ hides until the form closes, Cancel hides the form and drops what was typed, as
 does leaving the page and going back to it, and Save changes the description;
 of the recent events on the webhook page only the newest starts expanded, each
 expands and collapses, and Open leads to the event's own page; an event in the
-event log expands and collapses, and so do a delivery's attempts inside it; and
-at phone width the menu button opens and closes the mobile menu. It also fails
-if the browser reports a console warning or error, an uncaught exception, or
-anything the policy refused. `make check`
-and the image build lint it but do not run it, and `make test` leaves it out
-(its file is built only with the `browser` build tag). Run it with
+event log expands and collapses when its row's caret or its ID is clicked, and
+from the keyboard, but not when its ID is selected with the mouse, and a
+delivery's attempts inside it expand and collapse; and at phone width the menu
+button opens and closes the mobile menu. It also fails if the browser reports a
+console warning or error, an uncaught exception, or anything the policy refused.
+`make check` and the image build lint it but do not run it, and `make test`
+leaves it out (its file is built only with the `browser` build tag). Run it with
 `make test-browser` after changing `templates/` or `static/js/`: that builds
 `Dockerfile.browser`, which runs the test in a digest-pinned headless browser
 image, so the host needs no browser.

internal/server/alpine_browser_test.go

@@ -18,10 +18,13 @@ import (
 	"time"
 
 	"github.com/chromedp/cdproto/browser"
+	"github.com/chromedp/cdproto/dom"
+	"github.com/chromedp/cdproto/input"
 	"github.com/chromedp/cdproto/log"
 	"github.com/chromedp/cdproto/network"
 	"github.com/chromedp/cdproto/runtime"
 	"github.com/chromedp/chromedp"
+	"github.com/chromedp/chromedp/kb"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"gorm.io/gorm/clause"
@@ -40,6 +43,16 @@ const (
 	phoneWidth  = 390
 	phoneHeight = 844
 
+	// A window short enough that the event log scrolls with its last
+	// event expanded, and tall enough to show all of that event.
+	shortWidth  = 1024
+	shortHeight = 450
+
+	// A person's double- or triple-click: each press is held a tenth of a
+	// second, and the next press comes a quarter second after the release.
+	pressHeld     = 100 * time.Millisecond
+	betweenClicks = 250 * time.Millisecond
+
 	// olderBody is the body of the event received before the newest.
 	olderBody = "the older event"
 )
@@ -59,7 +72,7 @@ func TestAlpineRunsUnderTheSecurityPolicy(t *testing.T) {
 	t.Cleanup(srv.Close)
 
 	userID, _ := env.seedUser(t, "browser", "browser-password")
-	webhook, event, target := seedBrowserWebhook(t, env, userID)
+	webhook, older, event, target := seedBrowserWebhook(t, env, userID)
 
 	require.NoError(t, chromedp.Run(
 		ctx, setCookies(srv.URL, env.authCookies(t, userID, "browser")),
@@ -80,10 +93,10 @@ func TestAlpineRunsUnderTheSecurityPolicy(t *testing.T) {
 	checkCopy(ctx, t, page)
 	checkEntrypointEdit(ctx, t, page, page+"/events")
 	checkRecentEvents(ctx, t, page)
-	checkEventLog(ctx, t, page+"/events", event.ID, target.Name)
 	checkArchiveChoice(ctx, t, srv.URL+"/hooks/new", page)
 	checkNewWebhookTargets(ctx, t, env, srv.URL+"/hooks/new")
 	checkRefusedNewWebhook(ctx, t, srv.URL+"/hooks/new")
+	checkEventLog(ctx, t, page+"/events", event.ID, older.ID, target.Name)
 	checkMobileMenu(ctx, t, page)
 
 	assert.Empty(t, problems(), "the browser reported problems")
@@ -91,11 +104,11 @@ func TestAlpineRunsUnderTheSecurityPolicy(t *testing.T) {
 
 // seedBrowserWebhook seeds the webhook the browser test loads, owned by
 // userID: an entrypoint, two events, and a target whose delivery of the
-// newer event failed once with a 502. It returns the webhook, the newer
-// event and the target.
+// newer event failed once with a 502. It returns the webhook, the older
+// and the newer event, and the target.
 func seedBrowserWebhook(
 	t *testing.T, env *testEnv, userID string,
-) (*database.Webhook, *database.Event, *database.Target) {
+) (*database.Webhook, *database.Event, *database.Event, *database.Target) {
 	t.Helper()
 
 	webhook := env.seedWebhook(t, userID)
@@ -106,7 +119,7 @@ func seedBrowserWebhook(
 			Active:    true,
 		},
 	).Error)
-	env.seedEvent(t, webhook.ID, olderBody)
+	older := env.seedEvent(t, webhook.ID, olderBody)
 	event := env.seedEvent(t, webhook.ID, `{"hello":"browser"}`)
 	target := env.seedTarget(t, webhook.ID)
 	dlv := env.seedFailedDelivery(t, webhook.ID, event.ID, target.ID)
@@ -121,7 +134,7 @@ func seedBrowserWebhook(
 		},
 	).Error)
 
-	return webhook, event, target
+	return webhook, older, event, target
 }
 
 // startBrowser starts a headless browser for one test. It returns the
@@ -752,18 +765,30 @@ func checkRecentEvents(ctx context.Context, t *testing.T, url string) {
 		"the event's own page does not show its body")
 }
 
-// checkEventLog loads the event log and checks that clicking an event's
-// row expands it, that in there clicking its delivery shows the
-// delivery's attempts and clicking again hides them, and that clicking
-// the event's row again collapses it.
+// checkEventLog loads the event log and checks an event's row. Clicking
+// its ID expands the event, and in there clicking its delivery shows the
+// delivery's attempts and clicking again hides them. Clicking the row's
+// caret collapses the event, clicking it again expands it, and clicking
+// the ID again collapses it. While the event is expanded the row says so
+// and its caret is turned up, and while it is collapsed neither. It then
+// runs checkEventSelection on the log's last event, lastEventID, and
+// checkEventKeyboard on eventID.
 func checkEventLog(
-	ctx context.Context, t *testing.T, url, eventID, targetName string,
+	ctx context.Context,
+	t *testing.T,
+	url, eventID, lastEventID, targetName string,
 ) {
 	t.Helper()
 
-	// The event's row shows its ID, and its Resubmit form is in the part
-	// that expands. The delivery's row there shows the target's name.
-	eventRow := `//span[text()="` + eventID + `"]`
+	// The event's row shows its ID and ends with its caret, which turns
+	// up with Tailwind's rotate-180 class, and its Resubmit form is in
+	// the part that expands. The delivery's row there shows the target's
+	// name.
+	id := `//span[text()="` + eventID + `"]`
+	row := id + `/ancestor::div[@role="button"]`
+	caret := row + `//*[local-name()="svg"]`
+	caretUp := caret + `[contains(@class, "rotate-180")]`
+	caretDown := caret + `[not(contains(@class, "rotate-180"))]`
 	expanded := `form[action$="/` + eventID + `/resubmit"]`
 	deliveryRow := `//span[text()="` + targetName + `"]`
 	attempt := `//span[text()="Attempt 1"]`
@@ -772,8 +797,13 @@ func checkEventLog(
 
 	assert.True(t, hidden(ctx, expanded), "the event starts expanded")
 
-	click(ctx, t, eventRow)
-	assert.True(t, shown(ctx, expanded), "clicking the event does not expand it")
+	click(ctx, t, id)
+	assert.True(t, shown(ctx, expanded),
+		"clicking the event's ID does not expand it")
+	assert.True(t, shown(ctx, row+`[@aria-expanded="true"]`),
+		"the expanded event's row does not say it is expanded")
+	assert.True(t, shown(ctx, caretUp),
+		"the expanded event's caret does not turn up")
 
 	assert.True(t, hidden(ctx, attempt), "the delivery's attempts start shown")
 
@@ -785,9 +815,232 @@ func checkEventLog(
 	assert.True(t, hidden(ctx, attempt),
 		"clicking the delivery again does not hide its attempts")
 
-	click(ctx, t, eventRow)
+	click(ctx, t, caret)
 	assert.True(t, hidden(ctx, expanded),
-		"clicking the event again does not collapse it")
+		"clicking the caret does not collapse the event")
+	assert.True(t, shown(ctx, row+`[@aria-expanded="false"]`),
+		"the collapsed event's row does not say it is collapsed")
+	assert.True(t, shown(ctx, caretDown),
+		"the collapsed event's caret stays turned up")
+
+	click(ctx, t, caret)
+	assert.True(t, shown(ctx, expanded),
+		"clicking the caret again does not expand the event")
+
+	click(ctx, t, id)
+	assert.True(t, hidden(ctx, expanded),
+		"clicking the event's ID again does not collapse it")
+
+	checkEventSelection(ctx, t, url, lastEventID)
+	checkEventKeyboard(ctx, t, url, eventID)
+}
+
+// checkEventSelection loads the event log in a short window and checks
+// that selecting the ID of its last event, eventID, with the mouse leaves
+// the event as it was, and that its caret toggles it at once. Dragging
+// over the ID leaves the event collapsed, and the caret's click expands
+// it at once. With the page then scrolled to its end, a double-click on
+// the ID that goes on to drag along it, and a triple-click on it, each
+// leave the event expanded and select that ID. Had a click there
+// collapsed the event, the page would have got shorter and moved under
+// the pointer before the next click.
+func checkEventSelection(
+	ctx context.Context, t *testing.T, url, eventID string,
+) {
+	t.Helper()
+
+	id := `//span[text()="` + eventID + `"]`
+	row := id + `/ancestor::div[@role="button"]`
+	caret := row + `//*[local-name()="svg"]`
+	expanded := `form[action$="/` + eventID + `/resubmit"]`
+
+	var (
+		selected, state string
+		hasState        bool
+		scrolled        float64
+	)
+
+	// What is selected, and whether the event's row says it is expanded.
+	read := chromedp.Tasks{
+		chromedp.Evaluate(`window.getSelection().toString()`, &selected),
+		chromedp.AttributeValue(
+			row, "aria-expanded", &state, &hasState, chromedp.BySearch,
+		),
+	}
+
+	// A click on the ID toggles the event half a second after it, so a
+	// check that selecting the ID did not toggle it waits a second first.
+	settle := chromedp.Sleep(time.Second)
+
+	// The double-click and the triple-click each start with nothing
+	// selected, so that their first click waits to toggle the event.
+	clearSelection := chromedp.Evaluate(
+		`window.getSelection().removeAllRanges()`, nil,
+	)
+
+	require.NoError(t, chromedp.Run(
+		ctx, chromedp.EmulateViewport(shortWidth, shortHeight), loadPage(url),
+	))
+
+	selectText(ctx, t, id)
+	require.NoError(t, chromedp.Run(ctx, settle, read))
+	assert.Equal(t, eventID, selected, "the event's ID cannot be selected")
+	require.True(t, hasState, "the event's row does not say if it is expanded")
+	assert.Equal(t, "false", state, "selecting the event's ID expands it")
+
+	click(ctx, t, caret)
+	require.NoError(t, chromedp.Run(ctx, read))
+	assert.Equal(t, "true", state,
+		"clicking the caret does not expand the event at once")
+
+	// The row says it is expanded before its expanded part is shown, so
+	// the scroll waits for that part, to end at the expanded page's end.
+	require.True(t, shown(ctx, expanded),
+		"clicking the caret does not show the event's expanded part")
+
+	require.NoError(t, chromedp.Run(ctx, chromedp.Evaluate(
+		`window.scrollTo(0, document.body.scrollHeight); window.scrollY`,
+		&scrolled,
+	)))
+	require.Positive(t, scrolled, "the event log does not scroll")
+
+	require.NoError(t, chromedp.Run(ctx, clearSelection))
+	doubleClickAndDrag(ctx, t, id)
+	require.NoError(t, chromedp.Run(ctx, settle, read))
+	assert.Contains(t, selected, eventID,
+		"a double-click and drag does not select the event's ID")
+	assert.Equal(t, "true", state,
+		"a double-click and drag over the event's ID collapses it")
+
+	require.NoError(t, chromedp.Run(ctx, clearSelection))
+	tripleClick(ctx, t, id)
+	require.NoError(t, chromedp.Run(ctx, settle, read))
+	assert.Contains(t, selected, eventID,
+		"a triple-click does not select the event's ID")
+	assert.Equal(t, "true", state,
+		"a triple-click selecting the event's ID collapses it")
+}
+
+// checkEventKeyboard loads the event log and checks that Tab from the
+// page's Back link reaches the event's row, the first after it, and that
+// Enter then expands the event and Space collapses it.
+func checkEventKeyboard(
+	ctx context.Context, t *testing.T, url, eventID string,
+) {
+	t.Helper()
+
+	back := `//a[contains(text(), "Back to")]`
+	expanded := `form[action$="/` + eventID + `/resubmit"]`
+
+	var focused string
+
+	require.NoError(t, chromedp.Run(
+		ctx,
+		loadPage(url),
+		chromedp.Focus(back, chromedp.BySearch),
+		chromedp.KeyEvent(kb.Tab),
+		chromedp.Evaluate(`document.activeElement.textContent`, &focused),
+	))
+	require.Contains(t, focused, eventID,
+		"Tab from the Back link does not reach the event's row")
+
+	require.NoError(t, chromedp.Run(ctx, chromedp.KeyEvent(kb.Enter)))
+	assert.True(t, shown(ctx, expanded), "Enter does not expand the event")
+
+	require.NoError(t, chromedp.Run(ctx, chromedp.KeyEvent(" ")))
+	assert.True(t, hidden(ctx, expanded), "Space does not collapse the event")
+}
+
+// selectText selects the text of the element matching an XPath
+// expression as a person does with the mouse: pressing the button at the
+// text's start, moving to its end and releasing it there.
+func selectText(ctx context.Context, t *testing.T, xpath string) {
+	t.Helper()
+
+	left, right, y := textEnds(ctx, t, xpath)
+
+	require.NoError(t, chromedp.Run(
+		ctx, press(left, y, 1), drag(right, y), release(right, y, 1),
+	))
+}
+
+// doubleClickAndDrag double-clicks the start of the text of the element
+// matching an XPath expression, which selects its first word, and keeps
+// the button down to drag to the text's end, which selects it word by
+// word. It holds the button for a second, longer than a single click on
+// an event's row waits before it toggles the event.
+func doubleClickAndDrag(ctx context.Context, t *testing.T, xpath string) {
+	t.Helper()
+
+	left, right, y := textEnds(ctx, t, xpath)
+
+	require.NoError(t, chromedp.Run(
+		ctx,
+		press(left, y, 1), chromedp.Sleep(pressHeld), release(left, y, 1),
+		chromedp.Sleep(betweenClicks),
+		press(left, y, 2), drag(right, y), chromedp.Sleep(time.Second),
+		release(right, y, 2),
+	))
+}
+
+// tripleClick clicks three times in the middle of the text of the
+// element matching an XPath expression, as a person does to select a
+// whole line of text. The browser selects a word on the second click and
+// the whole paragraph on the third.
+func tripleClick(ctx context.Context, t *testing.T, xpath string) {
+	t.Helper()
+
+	left, right, y := textEnds(ctx, t, xpath)
+	x := (left + right) / 2
+
+	require.NoError(t, chromedp.Run(
+		ctx,
+		press(x, y, 1), chromedp.Sleep(pressHeld), release(x, y, 1),
+		chromedp.Sleep(betweenClicks),
+		press(x, y, 2), chromedp.Sleep(pressHeld), release(x, y, 2),
+		chromedp.Sleep(betweenClicks),
+		press(x, y, 3), chromedp.Sleep(pressHeld), release(x, y, 3),
+	))
+}
+
+// textEnds returns where on screen the text of the element matching an
+// XPath expression starts and ends, just inside its left and right
+// edges, and the height of its middle: in that order, the x of its
+// start, the x of its end, and the y of both.
+func textEnds(
+	ctx context.Context, t *testing.T, xpath string,
+) (float64, float64, float64) {
+	t.Helper()
+
+	var box *dom.BoxModel
+
+	require.NoError(t, chromedp.Run(
+		ctx, chromedp.Dimensions(xpath, &box, chromedp.BySearch),
+	))
+
+	// The content box's corners, clockwise from its top left.
+	return box.Content[0] + 1, box.Content[2] - 1,
+		(box.Content[1] + box.Content[5]) / 2
+}
+
+// press presses the left mouse button at x, y, as the nth click of a
+// double- or triple-click.
+func press(x, y float64, nth int64) *input.DispatchMouseEventParams {
+	return input.DispatchMouseEvent(input.MousePressed, x, y).
+		WithButton(input.Left).WithButtons(1).WithClickCount(nth)
+}
+
+// drag moves the pointer to x, y with the left mouse button down.
+func drag(x, y float64) *input.DispatchMouseEventParams {
+	return input.DispatchMouseEvent(input.MouseMoved, x, y).
+		WithButton(input.Left).WithButtons(1)
+}
+
+// release releases the left mouse button at x, y, as the nth click of a
+// double- or triple-click.
+func release(x, y float64, nth int64) *input.DispatchMouseEventParams {
+	return input.DispatchMouseEvent(input.MouseReleased, x, y).
+		WithButton(input.Left).WithClickCount(nth)
 }
 
 // The parts of the new webhook page the checks below find and click.

static/js/app.js

@@ -77,12 +77,36 @@ document.addEventListener("alpine:init", function () {
     window.Alpine.data("collapsible", function () {
         return {
             open: false,
+            // The timer of the toggle a single click is waiting to make.
+            pendingToggle: null,
             init() {
                 this.open = this.$root.hasAttribute("data-open");
             },
             toggle() {
                 this.open = !this.open;
             },
+            // Toggles on a click, except one that selects text, such as
+            // selecting an event's ID to copy it. A single click toggles
+            // only after 500 ms, the usual double-click interval, and the
+            // second press of a double- or triple-click cancels that (see
+            // cancelPendingToggle), so nothing moves under the pointer
+            // while it selects text.
+            toggleUnlessSelecting(event) {
+                if (
+                    event.detail === 1 &&
+                    window.getSelection().toString() === ""
+                ) {
+                    this.pendingToggle = setTimeout(() => this.toggle(), 500);
+                }
+            },
+            // Runs when the mouse button goes down, so the second press
+            // of a double- or triple-click cancels the toggle its first
+            // click is waiting to make, however long that press lasts.
+            cancelPendingToggle(event) {
+                if (event.detail > 1) {
+                    clearTimeout(this.pendingToggle);
+                }
+            },
             get closed() {
                 return !this.open;
             },

templates/source_logs.html

@@ -16,7 +16,8 @@
         <div class="divide-y divide-gray-100">
             {{range .Events}}
             <div class="p-4" x-data="collapsible">
-                <button type="button" class="btn-small w-full flex flex-wrap justify-between gap-2 text-left" @click="toggle">
+                <!-- Not a button element: browsers do not let a button's text be selected, and an event's ID must be. -->
+                <div role="button" tabindex="0" class="btn-small w-full flex flex-wrap justify-between gap-2" :aria-expanded="open" @mousedown="cancelPendingToggle" @click="toggleUnlessSelecting" @keydown.enter.prevent="toggle" @keydown.space.prevent="toggle">
                     <span class="flex flex-wrap items-center gap-3">
                         <span class="badge-info">{{.Method}}</span>
                         <span class="text-sm font-mono text-gray-700">{{.ID}}</span>
@@ -35,11 +36,12 @@
                         </span>
                         {{end}}
                         <span class="text-xs text-gray-400">{{.CreatedAt.Format "2006-01-02 15:04:05"}}</span>
-                        <svg class="w-4 h-4 text-gray-400 transition-transform" :class="caretClass" fill="none" stroke="currentColor" viewBox="0 0 24 24">
+                        <!-- The caret has no text to select, so a click on it toggles at once. -->
+                        <svg class="w-4 h-4 text-gray-400 transition-transform" :class="caretClass" @click.stop="toggle" fill="none" stroke="currentColor" viewBox="0 0 24 24">
                             <path stroke-linecap="round" stroke-linejoin="round" stroke-width="2" d="M19 9l-7 7-7-7"/>
                         </svg>
                     </span>
-                </button>
+                </div>
 
                 <div x-show="open" x-cloak class="mt-3 p-3 bg-gray-50 rounded-md">
                     <div class="mb-3 flex flex-wrap items-center justify-between gap-2">