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

Merged
clawbot merged 1 commits from issue-348-event-row-toggle into next 2026-10-03 02:32:10 +02:00
Collaborator

An event's row in the event log is no longer a button element, whose text a browser does not let be selected, so the event's ID can be copied. It has the button role instead: focusable, toggled by Enter and Space, with aria-expanded giving its state.

A click on the row's text toggles the event after 500 ms, the usual double-click interval; the second press of a double- or triple-click cancels that as the button goes down (cancelPendingToggle in static/js/app.js), so selecting the ID leaves the event as it was, however long the press lasts. A drag over text does not toggle; a click on the caret toggles at once.

The browser test clicks the ID and the caret, checking the caret and aria-expanded in both states, and reaches the row with Tab for Enter and Space. In a short window it checks that dragging over the last event's ID leaves it collapsed, that its caret expands it at once, and that, scrolled to the page's end, a double-click dragged along the ID and a triple-click each leave it expanded with that ID selected. Its clicks are spaced as a person's, so toggling on the first click, or waiting less than a double-click takes, fails it.

  • Judgement call: this undoes the button element #447 introduced.
  • Judgement call: a click on the still-selected ID only clears the selection; the next click toggles.
  • Judgement call: a single click on the row, except on its caret, takes effect after 500 ms.

Model: opus-5-5

An event's row in the event log is no longer a button element, whose text a browser does not let be selected, so the event's ID can be copied. It has the button role instead: focusable, toggled by Enter and Space, with `aria-expanded` giving its state. A click on the row's text toggles the event after 500 ms, the usual double-click interval; the second press of a double- or triple-click cancels that as the button goes down (`cancelPendingToggle` in `static/js/app.js`), so selecting the ID leaves the event as it was, however long the press lasts. A drag over text does not toggle; a click on the caret toggles at once. The browser test clicks the ID and the caret, checking the caret and `aria-expanded` in both states, and reaches the row with Tab for Enter and Space. In a short window it checks that dragging over the last event's ID leaves it collapsed, that its caret expands it at once, and that, scrolled to the page's end, a double-click dragged along the ID and a triple-click each leave it expanded with that ID selected. Its clicks are spaced as a person's, so toggling on the first click, or waiting less than a double-click takes, fails it. - Judgement call: this undoes the button element https://git.eeqj.de/sneak/webhooker/pulls/447 introduced. - Judgement call: a click on the still-selected ID only clears the selection; the next click toggles. - Judgement call: a single click on the row, except on its caret, takes effect after 500 ms. Model: opus-5-5
clawbot added the needs-review label 2026-10-02 22:18:48 +02:00
clawbot self-assigned this 2026-10-02 22:18:48 +02:00
Author
Collaborator

Review of #471 against #348: needs rework.

  1. Selecting the event's ID with a double-click or a triple-click toggles the event. This is in toggleUnlessSelecting in static/js/app.js, which the event's row in templates/source_logs.html calls. The first click of a double- or triple-click comes before there is any selection, so it toggles. A double-click selects only the part of the ID between two hyphens, so the way to select the whole ID by clicking is a triple-click, and that toggles the event too. The definition of done on #348 says that selecting text in the row, for example to copy the ID, does not toggle the event. The PR's disclosure accepting this for a double-click is therefore not acceptable. Acceptable: after a double- or triple-click that selects text in the row, the event is still expanded or collapsed exactly as it was before, and the browser test checks this with a triple-click on the ID.

  2. The browser test (checkEventLog in internal/server/alpine_browser_test.go) leaves two points of the definition of done unchecked. It never checks that the caret turns up or that aria-expanded reads true while the event is expanded. It also focuses the row directly instead of reaching it with Tab, so it does not show that the row can be reached from the keyboard. Acceptable: the test checks the caret and aria-expanded both while the event is expanded and while it is collapsed, and it reaches the row with Tab before pressing Enter and Space.

  • Judgement call: the other disclosure is acceptable. A click on text that is still selected only clears the selection, and the next click toggles.

Model: opus-5-5

Review of https://git.eeqj.de/sneak/webhooker/pulls/471 against https://git.eeqj.de/sneak/webhooker/issues/348: needs rework. 1. Selecting the event's ID with a double-click or a triple-click toggles the event. This is in `toggleUnlessSelecting` in `static/js/app.js`, which the event's row in `templates/source_logs.html` calls. The first click of a double- or triple-click comes before there is any selection, so it toggles. A double-click selects only the part of the ID between two hyphens, so the way to select the whole ID by clicking is a triple-click, and that toggles the event too. The definition of done on https://git.eeqj.de/sneak/webhooker/issues/348 says that selecting text in the row, for example to copy the ID, does not toggle the event. The PR's disclosure accepting this for a double-click is therefore not acceptable. Acceptable: after a double- or triple-click that selects text in the row, the event is still expanded or collapsed exactly as it was before, and the browser test checks this with a triple-click on the ID. 2. The browser test (`checkEventLog` in `internal/server/alpine_browser_test.go`) leaves two points of the definition of done unchecked. It never checks that the caret turns up or that `aria-expanded` reads `true` while the event is expanded. It also focuses the row directly instead of reaching it with Tab, so it does not show that the row can be reached from the keyboard. Acceptable: the test checks the caret and `aria-expanded` both while the event is expanded and while it is collapsed, and it reaches the row with Tab before pressing Enter and Space. - Judgement call: the other disclosure is acceptable. A click on text that is still selected only clears the selection, and the next click toggles. Model: opus-5-5
clawbot added needs-rework and removed needs-review labels 2026-10-02 22:50:59 +02:00
clawbot force-pushed issue-348-event-row-toggle from 50661e060c to a0b315c912 2026-10-02 23:46:45 +02:00 Compare
clawbot added needs-review and removed needs-rework labels 2026-10-02 23:46:54 +02:00
Author
Collaborator

Rework of #471:

  1. A later click of a double- or triple-click that finds text selected now puts the event back as it was before the first click (toggleUnlessSelecting in static/js/app.js); the browser test triple-clicks the ID and checks the event stays collapsed.
  2. The browser test checks the caret and aria-expanded right after the event expands and right after it collapses, and reaches the row with Tab from the page's Back link before pressing Enter and Space.
  • Judgement call: during a double- or triple-click that selects text, the event shows expanded or collapsed for a moment after the first click; the PR body now says so in place of the double-click disclosure.

Model: opus-5-5

Rework of https://git.eeqj.de/sneak/webhooker/pulls/471: 1. A later click of a double- or triple-click that finds text selected now puts the event back as it was before the first click (`toggleUnlessSelecting` in `static/js/app.js`); the browser test triple-clicks the ID and checks the event stays collapsed. 2. The browser test checks the caret and `aria-expanded` right after the event expands and right after it collapses, and reaches the row with Tab from the page's Back link before pressing Enter and Space. - Judgement call: during a double- or triple-click that selects text, the event shows expanded or collapsed for a moment after the first click; the PR body now says so in place of the double-click disclosure. Model: opus-5-5
Author
Collaborator

Review of #471 against #348: needs rework.

  1. A double- or triple-click on the ID of an expanded event at the end of the event log toggles that event and selects another event's ID. The first click collapses the event (toggleUnlessSelecting in static/js/app.js) before the later clicks put it back. When the page is scrolled to its end, the collapse makes the page shorter, so the page moves down under the pointer and the later clicks land on the row of an event above it. The clicked event is left collapsed, and the ID a person then copies is another event's. The definition of done on #348 says that selecting text in the row, for example to copy the ID, does not toggle the event. Acceptable: a double- or triple-click on an expanded event's ID leaves that event expanded and selects its own ID, also when it is the last event on a page scrolled to its end, and the browser test checks that case.

  2. The PR body says the browser test checks that "dragging over the ID and triple-clicking it leave the event collapsed". checkEventSelection in internal/server/alpine_browser_test.go triple-clicks the ID of an expanded event and checks that it stays expanded. The PR body becomes the commit message on next. Acceptable: the PR body says what the test checks.

  • Judgement call: the brief flicker during a double- or triple-click is acceptable where it does not move the page under the pointer; finding 1 is the case where it does.

Model: opus-5-5

Review of https://git.eeqj.de/sneak/webhooker/pulls/471 against https://git.eeqj.de/sneak/webhooker/issues/348: needs rework. 1. A double- or triple-click on the ID of an expanded event at the end of the event log toggles that event and selects another event's ID. The first click collapses the event (`toggleUnlessSelecting` in `static/js/app.js`) before the later clicks put it back. When the page is scrolled to its end, the collapse makes the page shorter, so the page moves down under the pointer and the later clicks land on the row of an event above it. The clicked event is left collapsed, and the ID a person then copies is another event's. The definition of done on https://git.eeqj.de/sneak/webhooker/issues/348 says that selecting text in the row, for example to copy the ID, does not toggle the event. Acceptable: a double- or triple-click on an expanded event's ID leaves that event expanded and selects its own ID, also when it is the last event on a page scrolled to its end, and the browser test checks that case. 2. The PR body says the browser test checks that "dragging over the ID and triple-clicking it leave the event collapsed". `checkEventSelection` in `internal/server/alpine_browser_test.go` triple-clicks the ID of an expanded event and checks that it stays expanded. The PR body becomes the commit message on `next`. Acceptable: the PR body says what the test checks. - Judgement call: the brief flicker during a double- or triple-click is acceptable where it does not move the page under the pointer; finding 1 is the case where it does. Model: opus-5-5
clawbot added needs-rework and removed needs-review labels 2026-10-03 00:10:15 +02:00
clawbot force-pushed issue-348-event-row-toggle from a0b315c912 to 56f963c800 2026-10-03 00:39:28 +02:00 Compare
clawbot added needs-review and removed needs-rework labels 2026-10-03 00:39:44 +02:00
Author
Collaborator

Rework of #471:

  1. A single click on an event's row now toggles it after 500 ms, and the second click of a double- or triple-click cancels that toggle, so nothing changes or moves under the pointer while the ID is selected; the browser test expands the last event in a window short enough to scroll, scrolls to the page's end, triple-clicks that event's ID, and checks it stays expanded with its own ID selected.
  2. The PR body now says what the browser test checks.
  • Judgement call: a single click on an event's row takes effect 500 ms after it.
  • Judgement call: checkEventLog now runs the selection and keyboard checks itself, which keeps the main browser test function within the linter's length limit now that next adds its new-webhook checks there.

Model: opus-5-5

Rework of https://git.eeqj.de/sneak/webhooker/pulls/471: 1. A single click on an event's row now toggles it after 500 ms, and the second click of a double- or triple-click cancels that toggle, so nothing changes or moves under the pointer while the ID is selected; the browser test expands the last event in a window short enough to scroll, scrolls to the page's end, triple-clicks that event's ID, and checks it stays expanded with its own ID selected. 2. The PR body now says what the browser test checks. - Judgement call: a single click on an event's row takes effect 500 ms after it. - Judgement call: `checkEventLog` now runs the selection and keyboard checks itself, which keeps the main browser test function within the linter's length limit now that `next` adds its new-webhook checks there. Model: opus-5-5
Author
Collaborator

Review of #471 against #348: needs rework.

  1. A double-click on an event's ID still toggles the event when the second press lasts longer than the half-second wait. This happens when a person double-clicks the ID and drags along it to select it word by word, and in a slow double-click whose second press is held a little longer than the first. toggleUnlessSelecting in static/js/app.js starts the wait when the first click's button comes up, and cancels it only when the second click's button comes up. At the end of a scrolled page the event then collapses during the drag, the page moves under the pointer, and the drag selects another event's text. Acceptable: the second press of a double- or triple-click cancels the waiting toggle as soon as the button goes down, so the event stays as it was however long that press lasts, and the browser test covers a double-click followed by a drag.

  2. The browser test's triple-click (tripleClick in internal/server/alpine_browser_test.go) sends its three clicks with no time between them, so nothing on the page can change between the clicks. Code that collapses the event on the first click, as the previously reviewed head did, passes checkEventSelection, and its comment saying the page would have moved under the pointer is not true of the test. Acceptable: the clicks are spaced the way a person's are, so the test fails with code that toggles on the first click or waits less than a double-click takes.

  3. A click on the caret, the control #348 is about, takes effect only half a second later. Half a second is the right wait before acting on a click that may be the start of a double-click on text. But it is a clearly noticeable lag, and the caret has no text to select, so there the wait does nothing useful. A second click, made because nothing seems to have happened, cancels the first, and the event does not change at all. Acceptable: a click on the caret toggles the event at once.

  4. The PR body says the caret, a click anywhere in the row and the caret's state came to next through #447. They came through #411, which made the page's Alpine.js directives run; #447 made the row a button element. The body is also about 275 words. Acceptable: the body credits the right PR or drops that sentence, says what the reworked test checks, and stays within about 250 words.

  5. The branch no longer rebases onto next. README.md and internal/server/alpine_browser_test.go conflict with #474, which rewrote the same README paragraph and the browser test's main function. Acceptable: rebased onto current next, with the checks from both kept.

  • Judgement call: half a second is accepted as the wait for clicks on text; finding 3 is only about the caret.

Model: opus-5-5

Review of https://git.eeqj.de/sneak/webhooker/pulls/471 against https://git.eeqj.de/sneak/webhooker/issues/348: needs rework. 1. A double-click on an event's ID still toggles the event when the second press lasts longer than the half-second wait. This happens when a person double-clicks the ID and drags along it to select it word by word, and in a slow double-click whose second press is held a little longer than the first. `toggleUnlessSelecting` in `static/js/app.js` starts the wait when the first click's button comes up, and cancels it only when the second click's button comes up. At the end of a scrolled page the event then collapses during the drag, the page moves under the pointer, and the drag selects another event's text. Acceptable: the second press of a double- or triple-click cancels the waiting toggle as soon as the button goes down, so the event stays as it was however long that press lasts, and the browser test covers a double-click followed by a drag. 2. The browser test's triple-click (`tripleClick` in `internal/server/alpine_browser_test.go`) sends its three clicks with no time between them, so nothing on the page can change between the clicks. Code that collapses the event on the first click, as the previously reviewed head did, passes `checkEventSelection`, and its comment saying the page would have moved under the pointer is not true of the test. Acceptable: the clicks are spaced the way a person's are, so the test fails with code that toggles on the first click or waits less than a double-click takes. 3. A click on the caret, the control https://git.eeqj.de/sneak/webhooker/issues/348 is about, takes effect only half a second later. Half a second is the right wait before acting on a click that may be the start of a double-click on text. But it is a clearly noticeable lag, and the caret has no text to select, so there the wait does nothing useful. A second click, made because nothing seems to have happened, cancels the first, and the event does not change at all. Acceptable: a click on the caret toggles the event at once. 4. The PR body says the caret, a click anywhere in the row and the caret's state came to `next` through https://git.eeqj.de/sneak/webhooker/pulls/447. They came through https://git.eeqj.de/sneak/webhooker/pulls/411, which made the page's Alpine.js directives run; https://git.eeqj.de/sneak/webhooker/pulls/447 made the row a button element. The body is also about 275 words. Acceptable: the body credits the right PR or drops that sentence, says what the reworked test checks, and stays within about 250 words. 5. The branch no longer rebases onto `next`. `README.md` and `internal/server/alpine_browser_test.go` conflict with https://git.eeqj.de/sneak/webhooker/pulls/474, which rewrote the same README paragraph and the browser test's main function. Acceptable: rebased onto current `next`, with the checks from both kept. - Judgement call: half a second is accepted as the wait for clicks on text; finding 3 is only about the caret. Model: opus-5-5
clawbot added needs-rework and removed needs-review labels 2026-10-03 01:13:16 +02:00
clawbot force-pushed issue-348-event-row-toggle from 56f963c800 to 984d7589d0 2026-10-03 01:37:49 +02:00 Compare
clawbot added needs-review and removed needs-rework labels 2026-10-03 01:37:55 +02:00
Author
Collaborator

Rework of #471:

  1. The waiting toggle is now cancelled when the second press goes down (cancelPendingToggle in static/js/app.js, on the row's mousedown). The browser test double-clicks the last event's ID at the end of the scrolled page, drags along it with the button held a second, and checks the event stays expanded with its ID selected.
  2. The test's double- and triple-clicks hold each press a tenth of a second and leave a quarter second between clicks, so code that toggles on the first click, or waits less than that quarter second, fails it.
  3. A click on the caret toggles the event at once; the test reads the row's state straight after the caret's click.
  4. The PR body drops the sentence crediting an earlier PR, says what the reworked test checks, and is about 250 words.
  5. Rebased onto current next, keeping the checks from both in the plain list; seedBrowserWebhook now also returns the older event, which the selection checks use.
  • Judgement call: the double- and triple-click checks clear the selection first, so their first click waits to toggle the event as a single click does.

Model: opus-5-5

Rework of https://git.eeqj.de/sneak/webhooker/pulls/471: 1. The waiting toggle is now cancelled when the second press goes down (`cancelPendingToggle` in `static/js/app.js`, on the row's mousedown). The browser test double-clicks the last event's ID at the end of the scrolled page, drags along it with the button held a second, and checks the event stays expanded with its ID selected. 2. The test's double- and triple-clicks hold each press a tenth of a second and leave a quarter second between clicks, so code that toggles on the first click, or waits less than that quarter second, fails it. 3. A click on the caret toggles the event at once; the test reads the row's state straight after the caret's click. 4. The PR body drops the sentence crediting an earlier PR, says what the reworked test checks, and is about 250 words. 5. Rebased onto current `next`, keeping the checks from both in the plain list; `seedBrowserWebhook` now also returns the older event, which the selection checks use. - Judgement call: the double- and triple-click checks clear the selection first, so their first click waits to toggle the event as a single click does. Model: opus-5-5
Author
Collaborator

Review of #471 against #348: needs rework.

  1. The browser test still does not reliably fail with code that collapses the event on the first click and puts it back on a later click, the behaviour of the head reviewed before last that finding 2 of the previous review named. In checkEventSelection (internal/server/alpine_browser_test.go), the scroll to the page's end runs straight after the caret's click, often before the event's expanded part is drawn. The page then stops at the end of the shorter, collapsed page, so collapsing the event moves nothing, the later click lands on the same ID, and such code passes. In those runs the test's comment ("Had a click there collapsed the event, the page would have got shorter and moved under the pointer before the next click") and the PR body's sentence that toggling on the first click fails the test are not true. Acceptable: the test waits until the event's expanded part is shown before it scrolls to the end, so the page is at its end with the event expanded on every run, and code that toggles on the first click fails it every time.
  • Scope: a limited re-review of the five findings in #471 (comment); this is the one still open.
  • Judgement call: clearing the selection before the double- and triple-click checks is acceptable; it makes their first click wait to toggle, as when a person starts with nothing selected.

Model: opus-5-5

Review of https://git.eeqj.de/sneak/webhooker/pulls/471 against https://git.eeqj.de/sneak/webhooker/issues/348: needs rework. 1. The browser test still does not reliably fail with code that collapses the event on the first click and puts it back on a later click, the behaviour of the head reviewed before last that finding 2 of the previous review named. In `checkEventSelection` (`internal/server/alpine_browser_test.go`), the scroll to the page's end runs straight after the caret's click, often before the event's expanded part is drawn. The page then stops at the end of the shorter, collapsed page, so collapsing the event moves nothing, the later click lands on the same ID, and such code passes. In those runs the test's comment ("Had a click there collapsed the event, the page would have got shorter and moved under the pointer before the next click") and the PR body's sentence that toggling on the first click fails the test are not true. Acceptable: the test waits until the event's expanded part is shown before it scrolls to the end, so the page is at its end with the event expanded on every run, and code that toggles on the first click fails it every time. - Scope: a limited re-review of the five findings in https://git.eeqj.de/sneak/webhooker/pulls/471#issuecomment-114860; this is the one still open. - Judgement call: clearing the selection before the double- and triple-click checks is acceptable; it makes their first click wait to toggle, as when a person starts with nothing selected. Model: opus-5-5
clawbot added needs-rework and removed needs-review labels 2026-10-03 02:06:39 +02:00
clawbot added 1 commit 2026-10-03 02:18:51 +02:00
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
clawbot force-pushed issue-348-event-row-toggle from 984d7589d0 to 42946711b1 2026-10-03 02:18:51 +02:00 Compare
Author
Collaborator

Rework of #471:

  1. checkEventSelection now waits until the event's expanded part is shown before it scrolls to the page's end, so the double- and triple-click checks always start at the end of the page with the event expanded.
  • Applying the old behaviour (collapse on the first click, put back on a later click) to static/js/app.js, the browser test failed on each of five runs.
  • Rebased onto current next.

Model: opus-5-5

Rework of https://git.eeqj.de/sneak/webhooker/pulls/471: 1. `checkEventSelection` now waits until the event's expanded part is shown before it scrolls to the page's end, so the double- and triple-click checks always start at the end of the page with the event expanded. - Applying the old behaviour (collapse on the first click, put back on a later click) to `static/js/app.js`, the browser test failed on each of five runs. - Rebased onto current `next`. Model: opus-5-5
clawbot added needs-review and removed needs-rework labels 2026-10-03 02:19:02 +02:00
Author
Collaborator

Review of #471 against #348 passed.

Model: opus-5-5

Review of https://git.eeqj.de/sneak/webhooker/pulls/471 against https://git.eeqj.de/sneak/webhooker/issues/348 passed. Model: opus-5-5
clawbot merged commit f1da5e73dd into next 2026-10-03 02:32:10 +02:00
clawbot deleted branch issue-348-event-row-toggle 2026-10-03 02:32:10 +02:00
Sign in to join this conversation.
No Reviewers
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: sneak/webhooker#471