Skip to content

one-key-formatting - Fix duplicate formatting#9865

Merged
fregante merged 8 commits into
mainfrom
deduplicate-delegate
Jul 20, 2026
Merged

one-key-formatting - Fix duplicate formatting#9865
fregante merged 8 commits into
mainfrom
deduplicate-delegate

Conversation

@fregante

@fregante fregante commented Jul 19, 2026

Copy link
Copy Markdown
Member

Mentioned in #9860 (comment)

Test URLs

  1. here
  2. visit commits tab
  3. come back
  4. select text in a field
  5. press *

Before

bug

After

closing remarks

@fregante fregante added the bug label Jul 19, 2026
@fregante
fregante marked this pull request as ready for review July 19, 2026 19:29
@fregante

fregante commented Jul 19, 2026

Copy link
Copy Markdown
Member Author

This works but I'll have to look for more such instances. observe still shouldn't be a problem because the rgh-seen-$id would be the same even if the function happened to be different

@fregante

Copy link
Copy Markdown
Member Author

Apparently just these

delegate(
'button[aria-label="Expand checks"]',
'click',
({delegateTarget}: DelegateEvent<MouseEvent, HTMLButtonElement>) => {
closestElement('section[aria-label="Checks"]', delegateTarget).classList.add('rgh-unclip-checks');
},
{signal, capture: true},
);

delegate(
[
...currentPrTitleSelectors,
'input#pull_request_title',
// Old `isCompare`
// TODO [2026-09-01]: Remove
'input#issue_title', // Old `isPR` view - TODO: Remove after legacy PR files view is removed
],
'input',
async ({delegateTarget}) => validatePrTitle(delegateTarget as HTMLInputElement),
{signal, passive: true},
);
// `isPR` - input is added to the DOM when user enters editing mode and removed when they exit it

delegate<TextField, 'keydown'>(selector, 'keydown', event => {

@fregante

Copy link
Copy Markdown
Member Author

@copilot extract those 2 "feature" delegate functions to the top level of their file

Copilot AI commented Jul 19, 2026

Copy link
Copy Markdown
Contributor

@copilot extract those 2 "feature" delegate functions to the top level of their file

Done in the latest commit. Both callbacks are now named top-level functions:

  • unclipChecks in unclip-checks.tsx
  • validatePrTitleDelegate in suggest-commit-title-limit.tsx

}],
'byo/no-inline-functions': ['error', {
selector:
"CallExpression[callee.name='delegate'] > :matches(ArrowFunctionExpression, FunctionExpression, CallExpression)",

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.

I had initially considered observe too, but:

  • even if it adds two animationstart listeners, only one is fired
  • we still have a number of features (like closing-remarks) that have one-off observe() calls and would need a refactoring

type TextField = HTMLTextAreaElement | HTMLInputElement;
type KeydownHandler = DelegateEventHandler<KeyboardEvent, TextField>;

function onFieldKeydown(selector: string | readonly string[], callback: KeydownHandler, signal: AbortSignal): void {

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.

Dropped internal onFieldKeydown helper because it made deduplication more difficult. A memoized callback filter is slightly easier to follow.

@fregante
fregante merged commit 9a28ac8 into main Jul 20, 2026
13 checks passed
@fregante
fregante deleted the deduplicate-delegate branch July 20, 2026 10:49
@fregante

Copy link
Copy Markdown
Member Author

All good. It's might drop the lint rule once feature unloading is restored, but I still think it makes sense as a general rule regardless of that.

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