Skip to content

clean-conversation-headers - Fix base branch detection - #9957

Open
Sebastien-Ahkrin wants to merge 2 commits into
refined-github:mainfrom
Sebastien-Ahkrin:fix/clean-conversation-header
Open

clean-conversation-headers - Fix base branch detection #9957
Sebastien-Ahkrin wants to merge 2 commits into
refined-github:mainfrom
Sebastien-Ahkrin:fix/clean-conversation-header

Conversation

@Sebastien-Ahkrin

@Sebastien-Ahkrin Sebastien-Ahkrin commented Aug 8, 2026

Copy link
Copy Markdown
Contributor

Closes: #9952

This PR is here to solve the issues, that sometime the wrong ref is highlighted (posted by @fregante)
This bug happened when Edit title was clicked before RGH has finished loading.

I tested it on Chromium, Firefox and Safari.

What I did

We were picking the first element matching [class^="PullRequestBranchName"] as the base branch, without checking if the head was also in the DOM. Now, we check in the observe if both branches are in the DOM.

Test URLs

Screenshot / GIF

GIF from my Chromium
CleanShot 2026-08-08 at 22 40 54

Tell me if this looks correct for you.

@github-actions github-actions Bot changed the title fix(clean-conversation-headers): wait for both branch tags fix(clean-conversation-headers): wait for both branch tags Aug 8, 2026
@github-actions github-actions Bot added the bug label Aug 8, 2026
@Sebastien-Ahkrin
Sebastien-Ahkrin marked this pull request as ready for review August 8, 2026 20:55
@fregante

fregante commented Aug 9, 2026

Copy link
Copy Markdown
Member

Thank you! The selector should eventually just select the head branch, not use :has() due to performance concerns.

Inside cleanPrHeader you can then use const summaryRow = closestElement('some selector', headRef)

@Sebastien-Ahkrin
Sebastien-Ahkrin force-pushed the fix/clean-conversation-header branch from 19cf792 to 6a088b0 Compare August 9, 2026 10:12
@github-actions

github-actions Bot commented Aug 9, 2026

Copy link
Copy Markdown

Please avoid force-pushing to this PR. It makes it harder for reviewers to track what changed since the last review.

Just push new commits on top of the branch instead — PRs are squashed on merge, so the commit history doesn't need to be clean.

@Sebastien-Ahkrin

Copy link
Copy Markdown
Contributor Author

Sorry for the force-push, wanted to reword a commit for typo.
My latest commit is about to change the old selector with new one on observe [class^="PullRequestBranchName"] ~ div [class^="PullRequestBranchName"].

Result ended up to be the same as previous CF the gif.
CleanShot 2026-08-09 at 12 10 33

@Sebastien-Ahkrin

Sebastien-Ahkrin commented Aug 10, 2026

Copy link
Copy Markdown
Contributor Author

Do I have to do something before merging @fregante ?
If yes, i don't know what ! can you tell me ? I do not see something on the collaborating file about that

fregante
fregante previously approved these changes Aug 11, 2026

@fregante fregante left a comment

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.

Thank you!

@fregante fregante changed the title fix(clean-conversation-headers): wait for both branch tags clean-conversation-headers - Fix base branch detection Aug 11, 2026
@Sebastien-Ahkrin
Sebastien-Ahkrin force-pushed the fix/clean-conversation-header branch from 6a088b0 to 2145773 Compare August 11, 2026 11:29
@fregante
fregante dismissed their stale review August 11, 2026 11:47

Force pushed

@Sebastien-Ahkrin

Copy link
Copy Markdown
Contributor Author

@fregante Sorry, I clicked the wrong button in my Github Interface and it rebased my branch on top of main with a force-push ..
The PR should now be up to date with main, no changes to the actual source code.

Let me know if something looks off !

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.

clean-conversation-headers - head ref is mistakenly highlighted

2 participants