Skip to content

table-input - Restore feature in PR page - #10002

Open
kovsu wants to merge 4 commits into
refined-github:mainfrom
kovsu:table-input-files-changed
Open

table-input - Restore feature in PR page#10002
kovsu wants to merge 4 commits into
refined-github:mainfrom
kovsu:table-input-files-changed

Conversation

@kovsu

@kovsu kovsu commented Aug 25, 2026

Copy link
Copy Markdown
Member

Test URLs

Any PR page

Screenshot

.rgh-tic
  → <details-menu>
  → <details>                  ← the listener is now attached here (via `base`); the event is handled ✓
  → …editor layers…
  → div[data-inline-markers]   ← GitHub calls stopPropagation() here ✂️
  → React root → document      ← the old document-level `delegate` listener never hears it ✗
Before After
CleanShot.2026-08-25.at.15.45.27.mp4
CleanShot.2026-08-25.at.16.04.43.mp4

The new PR "Files changed" view wraps inline comment editors in `div[data-inline-markers]`, whose React handler stops click propagation, so the document-level `delegate` never received the squares' clicks: the menu opened and closed but nothing was inserted. Listen on the injected `<details>` instead, below the interception point.

Also remove a stray "," text node inside the `<details>`, left over from the JSX conversion in refined-github#9845.
@kovsu kovsu changed the title table-input - Fix table insertion on the new "Files changed" view table-input - Restore feature in PR page Aug 25, 2026
@kovsu
kovsu requested a review from fregante August 25, 2026 08:13
Comment thread source/features/table-input.tsx
@fregante fregante added the bug label Aug 25, 2026
Comment thread source/features/table-input.tsx Outdated
@kovsu
kovsu requested a review from fregante August 25, 2026 15:45
import {withTooltipRef} from '../components/tooltip.js';

function addTable({delegateTarget: square}: DelegateEvent<MouseEvent, HTMLButtonElement>): void {
closestElement('details', square).open = false;

@fregante fregante Aug 27, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I saw your commit title, but I don't see the bug that this and capture are fixing. What's your browser? The table is inserted and the field is focused in Safari:

Image

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We need inline comments explaining what code does, why, and where

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Development

Successfully merging this pull request may close these issues.

2 participants