-
Notifications
You must be signed in to change notification settings - Fork 7.6k
Remove basePadding from indentwrap hack #12110
Conversation
tagging @swmitra |
Same issue. Here is my original question and also temporary fix for now. https://groups.google.com/forum/?hl=en#!topic/brackets-dev/6ri52cF2FB8 On Thu, Jan 21, 2016 at 10:24 AM, Randy Edmunds [email protected]
|
@petetnt has anyone figured out when this gap appeared and why is started happening? I only noticed it fairly recently. If we understood what introduced the problem we could address it where it was introduced instead of hacking the style sheet. |
@petetnt, never mind, I see what the problem is. I didn't realize that ugly Code Mirror word wrap hack made it into the core code. We really should have made that a preference. Actually, it should not even be a preference. I think that functionality was available in an extension and it never worked correctly, even in the extension. It should not be in the core code IMHO. Sorry I didn't see it sooner or I would have shared my opinion sooner. |
FWIW I think the padding has been working pretty well (sans the 4px space issue this PR is trying to account for). I do think that the hack is pretty ugly too, but indent wrapping is much more crucial IMHO. |
In #12184 there's another case where the gutter shows up (and screws the fold marks too) with this fix. The thing is that I can't replicate it on Windows 10, 8.1 nor Mac OS, all with same version of Brackets Shell and same commit. |
I too cant replicate the |
LGTM, even I can not replicate the gap issue. |
@abose Can we get this fix in for 1.7 ? |
@redmunds do you have any idea what causes the gutter thing yet? I have tried this on 10+ computers and I still haven't been able to replicate the issue you were having above 😕 |
@petetnt No, I haven't had time to look at that. But, I think it's an improvement. |
@@ -27,6 +27,7 @@ | |||
|
|||
> * { | |||
text-indent: 0px; | |||
margin-left: -4px; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@petetnt could this be done as part of the hack itself as opposed to stylesheet? this way the value of the shift (4px
) will be shared and not duplicated.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@petetnt Not sure how to fix it in the hack itself, but I found out that just removing padding: 0 @code-padding 0 0;
on line 26 also fixes the issue.
b461a1a
to
3ff302e
Compare
Made some changes to this, 3ff302e now contains the compensation in the padding hack itself. Not sure if it's the most optimal solution either but seems to work for me. |
Build failed to the |
@petetnt I confirmed that your fix is good with the default "show line numbers" on. If you turn off line numbers, then all lines are shifted to the left and your change does not fix in that case. So I believe we also need to adjust |
Thanks for the feedback @RaymondLim , I'll get that fixed ASAP 👍 |
Somewhat related: #12630 |
Signed-off-by: petetnt <[email protected]>
3ff302e
to
966b466
Compare
@RaymondLim adjusting However, removing the Does it work for you / others? |
@petetnt Sorry, I was wrong to say that I don't have any other changes. Actually, I do have the following change in Editor.js that I believe is from your previous commit.
|
Yeah, but that is not needed anymore. Together with #12630 all of the issues should be resolved. (The PR's solve different issues) |
LGTM, if we'll find more issue we can look at them in another PR. |
As noted by @redmunds on Slack there's an slightly ankward gap between the gutters and the text
This is due to the indent-wrap hack in #11909: the texts are pushed 4 pixels back by the hack. This PR compensates for that by pulling the editor back -4 pixels: