`esc-to-cancel` - Restore feature on PRs by anasm266 · Pull Request #9280 · refined-github/refined-github

anasm266 · 2026-04-25T17:50:03Z

Test URLs

Merged PR sandbox#1

Demo gif

(

fix_tested.mp4

fregante · 2026-04-25T18:38:28Z

+
+function findCancelButton(field: HTMLInputElement): HTMLButtonElement | HTMLAnchorElement | undefined {
+	for (const titleContainerSelector of titleContainerSelectors) {
+		const titleContainer = field.closest(titleContainerSelector);


No loop needed, just pass the whole array to .closest()

fregante · 2026-04-25T18:38:55Z

+function findCancelButton(field: HTMLInputElement): HTMLButtonElement | HTMLAnchorElement | undefined {
+	for (const titleContainerSelector of titleContainerSelectors) {
+		const titleContainer = field.closest(titleContainerSelector);
+		if (!titleContainer) {


closest() must/will find the element, so no condition needed here

fregante · 2026-04-25T18:40:50Z

+			continue;
+		}
+
+		const localCancelButton = [...titleContainer.querySelectorAll<HTMLButtonElement>('button:not([disabled])')]


$$() already selects, returns an array, and is properly typed:

Suggested change

const localCancelButton = [...titleContainer.querySelectorAll<HTMLButtonElement>('button:not([disabled])')]

const localCancelButton = $$('button:not([disabled])')

fregante · 2026-04-25T18:42:43Z

+		}
+	}
+
+	return $optional<HTMLButtonElement | HTMLAnchorElement>('.js-cancel-issue-edit');


This cannot be optional. The button must be found. If it's not found, the selectors are wrong and/or GitHub was updated. In both cases we want an error to be thrown ($() will throw, $optional() doesn't)

fregante · 2026-04-25T18:44:02Z

 			|| field.hasAttribute('aria-autocomplete')
 			// Classic autocomplete dropdown
-			|| elementExists('.suggester', field.form!)
+			|| elementExists('.suggester', field.form ?? undefined)


Don't make unnecessary changes

fregante · 2026-04-25T18:44:54Z

+		}
+
+		const cancelButton = findCancelButton(event.delegateTarget);
+		if (!cancelButton) {


As mentioned, the cancel button must be found

fregante · 2026-04-25T18:45:48Z

Note that untested AI PRs will result in a block. So make sure it works and ideally record a screencast or gif (with LICEcap if you use macos for example)

anasm266 · 2026-04-25T19:40:58Z

fix_tested.mp4

fregante

Thank you! Looks reasonable already

fregante · 2026-04-26T04:54:13Z

+
+function findCancelButton(field: HTMLInputElement): HTMLButtonElement | HTMLAnchorElement {
+	const titleContainer = field.closest(titleContainerSelectors.join(','))!;
+	return $$('button:not([disabled])', titleContainer)


Without changing the order of operations, we have to temporarily use $$optional here or else it might throw on the old version.

SunsetTechuila · 2026-04-26T13:04:02Z

+
+function findCancelButton(field: HTMLInputElement): HTMLButtonElement | HTMLAnchorElement {
+	const titleContainer = field.closest(titleContainerSelectors);
+	return $$optional('button:not([disabled])', titleContainer ?? undefined)


@fregante allow baseElements in select-dom to be null 🙏

I follow sindresorhus/meta#7

I guess an exception can be made since the DOM has a lot of nulls. PR welcome to explore it further

Co-authored-by: Copilot <[email protected]>

fregante · 2026-04-27T03:44:35Z

+			// Old view -- TODO: Remove after legacy PR files view is removed
+			'.js-cancel-issue-edit',
+		]);
+		if (cancelButton.textContent.trim() !== 'Cancel') {


We have assertNodeContent

Which works only with text nodes or elements containing only them

Therefore the code can be adjusted to use it, what's the issue? cancelButton.firstChild probably.

The issue is that it would be cancelButton.firstChild.firstChild

Or cancelButton.querySelector('span[data-component="text"]'). I like neither

fregante · 2026-04-27T03:45:46Z

 }

-// TODO: Drop in March 2025, implemented by GitHub
-// https://github.com/refined-github/refined-github/pull/7892


We still want a TODO comment to check if GitHub is already handling the keypress or not. September 2026 would be good, then every 6 months or so.

Co-authored-by: fregante <[email protected]>

Co-authored-by: Grigory <[email protected]> Co-authored-by: fregante <[email protected]>

Restore esc-to-cancel on new PR header

bb0d468

github-actions Bot added the bug label Apr 25, 2026

github-actions Bot changed the title ~~Fix esc-to-cancel on new PR header~~ Fix esc-to-cancel on new PR header Apr 25, 2026

fregante requested changes Apr 25, 2026

View reviewed changes

Address PR review feedback

ebfd93d

anasm266 requested a review from fregante April 25, 2026 19:41

anasm266 marked this pull request as ready for review April 26, 2026 03:54

fregante approved these changes Apr 26, 2026

View reviewed changes

fregante reviewed Apr 26, 2026

View reviewed changes

Comment thread source/features/esc-to-cancel.tsx Outdated

Address review feedback

c7f28fd

SunsetTechuila reviewed Apr 26, 2026

View reviewed changes

Comment thread source/features/esc-to-cancel.tsx Outdated

SunsetTechuila changed the title ~~Fix esc-to-cancel on new PR header~~ esc-to-cancel - Restore feature on PRs Apr 26, 2026

SunsetTechuila reviewed Apr 26, 2026

View reviewed changes

SunsetTechuila and others added 6 commits April 26, 2026 20:18

comments

cecf80a

Co-authored-by: Copilot <[email protected]>

Remove unnecessary normalizeText in esc-to-cancel

da215c6

drop issues

749f75c

find cancel button differently

52c998a

update test

ffb9918

better names

0eff266

SunsetTechuila approved these changes Apr 26, 2026

View reviewed changes

fregante reviewed Apr 27, 2026

View reviewed changes

update test url

a9d594d

Co-authored-by: fregante <[email protected]>

fregante merged commit a49635e into refined-github:main Apr 30, 2026
10 checks passed

Copilot AI pushed a commit that referenced this pull request Apr 30, 2026

esc-to-cancel - Restore feature on PRs (#9280)

9001914

Co-authored-by: Grigory <[email protected]> Co-authored-by: fregante <[email protected]>

	const localCancelButton = [...titleContainer.querySelectorAll<HTMLButtonElement>('button:not([disabled])')]
	const localCancelButton = $$('button:not([disabled])')

Uh oh!

Conversation

anasm266 commented Apr 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test URLs

Demo gif

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fregante commented Apr 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

anasm266 commented Apr 25, 2026

Uh oh!

fregante left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

3 participants

anasm266 commented Apr 25, 2026 •

edited

Loading

fregante commented Apr 25, 2026 •

edited

Loading