Skip to content

closing-remarks - Support new PR merge box - #8164

Merged
fregante merged 5 commits into
mainfrom
closing-remarks-merge-box
Dec 18, 2024
Merged

closing-remarks - Support new PR merge box#8164
fregante merged 5 commits into
mainfrom
closing-remarks-merge-box

Conversation

@fregante

@fregante fregante commented Dec 18, 2024

Copy link
Copy Markdown
Member

Test URLs

Any open PR, like this page

Screenshot

Gif also shows other auto-deletion working, but that's a separate PR

merging 2

@fregante fregante added the bug label Dec 18, 2024
@fregante
fregante marked this pull request as ready for review December 18, 2024 09:22

const selector = typeof selectors === 'string' ? selectors : selectors.join(',\n');
const seenMark = 'rgh-seen-' + getCallerID();
const seenMark = 'rgh-seen-' + getCallerID(ancestor);

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 don’t even know how this worked until now. The observer's callback was only called once because the generated rgh-seen class name was identical and successive callbacks would fail.

The observer should likely be refactored to make sure all listeners are called (probably just needs a await Promise.resolve() before removing the class but alas)

Comment thread source/github-events/on-pr-merge.ts Outdated
Comment on lines +15 to +20
await oneEvent(document.body, 'click', {signal, filter: ({target}: Event) => {
const clicked = target as HTMLElement;
// TODO: Drop `js-merge-commit-button` in May 2025
return Boolean(clicked.closest('.js-merge-commit-button'))
|| /^Confirm .+ merge$/i.test(clicked.textContent ?? '');
}});

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.

The selector I found was not specific enough, it also matched the initial "Merge and squash" button.

But actually with :has(textarea) this appears to be enough.

I'll wait for fregante/delegate-it#55 to maybe restore an extra textContent check

@fregante fregante left a comment

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.

Took so long

@fregante
fregante enabled auto-merge (squash) December 18, 2024 09:42
@fregante
fregante merged commit cd3886d into main Dec 18, 2024
@fregante
fregante deleted the closing-remarks-merge-box branch December 18, 2024 09:43
@fregante fregante mentioned this pull request May 1, 2026
1 task
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.

1 participant