Skip to content

prevent-link-loss - Mount banner outside the fieldset #9970

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Sebastien-Ahkrin wants to merge 3 commits into
base: main
Choose a base branch
from
+16 −3
Open

prevent-link-loss - Mount banner outside the fieldset #9970

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
31d939e
fix(prevent-link-loss): mount banner outside the fieldset
Sebastien-Ahkrin Aug 11, 2026
f4585bb
refactor: do not create a new div
Sebastien-Ahkrin Aug 11, 2026
de823da
fix: mount banner outside fieldset only on new markdown editor
Sebastien-Ahkrin Aug 11, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 16 additions & 3 deletions source/features/prevent-link-loss.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,24 @@ function attach(field: HTMLTextAreaElement): void {
// Editing PR body
'.CommentBox',

@fregante fregante Aug 11, 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.

Can you check the PR Conversation tab? New comment, editing comment, editing PR body. Those are the ones that use the old markup so this PR might break them

@Sebastien-Ahkrin Sebastien-Ahkrin Aug 11, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

PR Conversation Tab (Globally Here)

I tested on this PR. I think, the PR Conversation Tab is here

Gif of tests are here

New Comment

CleanShot 2026-08-11 at 22 57 18

Editing Comment

CleanShot 2026-08-11 at 22 59 38

Editing PR Body (I already have one link that trigger the Extension)

CleanShot 2026-08-11 at 22 58 51

@Sebastien-Ahkrin Sebastien-Ahkrin Aug 11, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Does this is the correct tab to test ?

@fregante fregante Aug 11, 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.

Yes. The margins are bad though, this fix is no longer being applied

@fregante fregante Aug 11, 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.

Note that the selector targets "the old style" via file-attachment element. This is preferred over using "isPR" checks because the PR view could be updated soon and break again.

@Sebastien-Ahkrin Sebastien-Ahkrin Aug 11, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Sorry, I put the pr on draft, and re ask when everything is good

@Sebastien-Ahkrin Sebastien-Ahkrin Aug 11, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Ok, a better way to solve this is to only change the mounting strategy for the new markdown editor (the one that has the height collapse bug from this issue and keep the origin behavior for the old markup (new comment, editing a comment, and editing the PR body). After testing, the bug only seems to happen on the new editor, so this should be safer now.

Thanks for catching those bugs I didn't saw theses !

Screenshots of each views

New issue form

CleanShot 2026-08-12 at 00 20 57

New comment form

CleanShot 2026-08-12 at 00 21 12

New review form

CleanShot 2026-08-12 at 00 21 27

### New review comment form

CleanShot 2026-08-12 at 00 21 42

### New comment on PR

CleanShot 2026-08-12 at 00 21 59

Editing comment on PR

CleanShot 2026-08-12 at 00 22 29

Editing PR Body (with the margin bug)

CleanShot 2026-08-12 at 00 22 47

], field);

if (target instanceof HTMLFieldSetElement) {
// On the new Markdown editor mounting a child directly inside the `fieldset` collapses
// its height to 0, hiding the textarea entirely. Mounting as a sibling instead avoids the issues.
// https://github.com/refined-github/refined-github/issues/9955
mount(Banner, {
target: target.parentElement!,
anchor: target.nextSibling ?? undefined, props: {field},
});

return;
}

// Old Markdown editor doesn't have this bug, so we can keep mounting inside it.
// This also preserve the extra margin for old views (PR)
mount(Banner, {
target,
props: {
field,
},
props: {field},
});
}

Expand Down
Loading