prevent-link-loss - Mount banner outside the fieldset - #9970
prevent-link-loss - Mount banner outside the fieldset#9970Sebastien-Ahkrin wants to merge 3 commits into
prevent-link-loss - Mount banner outside the fieldset#9970Conversation
prevent-link-loss): mount banner outside the fieldset
|
Tell me if something looks wrong ! |
|
Thanks! No conventional commit here by the way |
Oh ok ! Sorry, do you want me to rewrite my commits or its ok for this PR ? Sorry ! |
|
No the commits don't matter but the PR title does 😭 |
prevent-link-loss): mount banner outside the fieldsetprevent-link-loss - Mount banner outside the fieldset
| @@ -14,11 +14,10 @@ function attach(field: HTMLTextAreaElement): void { | |||
| // Editing PR body | |||
| '.CommentBox', | |||
There was a problem hiding this comment.
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
There was a problem hiding this comment.
Does this is the correct tab to test ?
There was a problem hiding this comment.
Yes. The margins are bad though, this fix is no longer being applied
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Sorry, I put the pr on draft, and re ask when everything is good
There was a problem hiding this comment.
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.
Plus, I'm sorry for the time that this PR is taking to you ..
Ok sorry ! I updated it then ! |










prevent-link-losswas mounting its banner directly inside the closestfieldsetor.CommentBoxelement. That result on github to a markdown editor with height at 0 hiding the comment textarea.Closes: #9955
Tested on Safari, Chrome and Firefox
Test done to make my fix
The height will be 0 if we put the Banner direclty inside the target.
Will result to :
target: FIELDSET MarkdownEditor-module__fieldSet__RU0NL height before: 215.38284301757812 height after: 0 textarea rect after: DOMRectCF : height after: 0
So the easier fix was to mount the banner as a sibling of the target instead of inside it
Test URLs
I will test with this link in my GIF :
https://github.com/refined-github/refined-github/pull/6954/commits/32d1c8b2e1b6971709fe273cfdd1f959b51e8d85issuesandbox#3Screenshot / GIF
Sorry for the screens they are at zoom 100% not 200% to see the entire feature. Hope this is still good
New issue form
New comment form
New review form
New review comment form