fix: text overlay in replace input#162036
Conversation
Signed-off-by: Yue Yang <g1enyy0ung@gmail.com>
|
@microsoft-github-policy-service agree |
|
|
||
| public set width(newWidth: number) { | ||
| this.inputBox.paddingRight = this.cachedOptionsWidth; | ||
| this.inputBox.width = newWidth; |
There was a problem hiding this comment.
Here I remove this line because set paddingRight(paddingRight: number) already calculates the input width with the right padding. Setting the width again is redundant.
| display: inline-block; | ||
| vertical-align: middle; | ||
| width: 100% !important; | ||
| width: 100%; |
There was a problem hiding this comment.
This change and the below changes aim to reduce unnecessary width settings. The inline width will be overridden by width: 100% !important;, so these operations are also redundant.
|
Is there an open issue for this? I think I remember one but can't find it. If not, please open one. |
Linked the relevant issue |
|
@roblourens @andreamah Will this PR be included in the October release? |
Unfortunately, we weren't able to get to reviewing this one in time for October, apologies for that! :( |
Never mind. Hopefully this fix will be reviewed and merged in November. :) |
|
|
||
| setWidth(newWidth: number): void { | ||
| this.width = newWidth; | ||
| this.domNode.style.width = this.width + 'px'; |
There was a problem hiding this comment.
I'm a bit unsure on why this is redundant? Wouldn't we want to keep re-setting the findInput's width for include/exclude?
There was a problem hiding this comment.
@andreamah Perhaps these screenshots can help to understand why this line and below are redundant. If the .monaco-findInput has the width: 100% property (changes above 👆), setting the dom width manually will override it.
These two parts contradict each other, the past code used the !important rule, then manually setting the widths is a redundant operation, as they will be overridden. The opposite is also true, so here we just need to remove the !important and remove the manual setting of the dom width to allow the input width to scale automatically.
Signed-off-by: Yue Yang <g1enyy0ung@gmail.com>
andreamah
left a comment
There was a problem hiding this comment.
LGTM! Thanks for the contribution!
Signed-off-by: Yue Yang g1enyy0ung@gmail.com
This PR tries to fix the below text overlay problem in the replace input:
After:
I will add the explanation in the comments.
Fixes #151624