[CLOSED] Fix Issue #7283: Padding with line numbers turned off is broken

[core-ai-bot]

Issue by lkcampbell
Friday Apr 25, 2014 at 07:13 GMT
Originally opened as adobe/brackets#7641

---

Fix Issue #7283: Padding with line numbers turned off is broken.

---

lkcampbell included the following code: https://github.com/adobe/brackets/pull/7641/commits

[core-ai-bot]

Comment by lkcampbell
Friday Apr 25, 2014 at 07:35 GMT

---

@TomMalbran, hold up I added too much padding with the line numbers. Working on it now.

[core-ai-bot]

Comment by TomMalbran
Friday Apr 25, 2014 at 07:38 GMT

---

Yep. I noticed. The class should only be applied if the line numbers are hidden, probably only once after the html is ready from the EditorOptionHandlers

[core-ai-bot]

Comment by lkcampbell
Friday Apr 25, 2014 at 08:45 GMT

---

@TomMalbran, okay, ready now.

[core-ai-bot]

Comment by TomMalbran
Friday Apr 25, 2014 at 09:04 GMT

---

One final thing. Does the padding seems enough for you, with show line numbers off? It seems to me that a bit larger might look better. cc@larz0 in case you want to take a look.

I guess I should assign this PR to myself. jeje

[core-ai-bot]

Comment by lkcampbell
Friday Apr 25, 2014 at 22:07 GMT

---

@TomMalbran, if we go strictly off of the original bug, any padding that keeps the selected file arrow head from overlapping on the editor text is sufficient. If I go off of my own needs, any padding allows me to print my ruler's zero tick mark in full is sufficient. By both of these criteria, the padding is large enough.

But yeah, let's see what@larz0 has to say as well.

[core-ai-bot]

Comment by TomMalbran
Friday Apr 25, 2014 at 22:09 GMT

---

For usability is large enough, but from a design point of view it looks too close to the border or to the arrow. But it might also be just me used to have the line numbers to create extra spacing.

[core-ai-bot]

Comment by lkcampbell
Saturday Apr 26, 2014 at 15:33 GMT

---

@TomMalbran, code review changes committed. It's up to you if you want to merge now or wait on input from@larz0 on the padding width.

[core-ai-bot]

Comment by TomMalbran
Saturday Apr 26, 2014 at 18:34 GMT

---

The code looks great, and actually we are using the same padding as before it was broken, so I think that it is fine.

The is one other issue I just realized. Issue #7283 came after fixing adobe/brackets#5293 by using a darker color for the line numbers. With this fix, that issue would be broken in the case of no line numbers, but this could be solved using a box-shadow:

.show-line-padding .CodeMirror-activeline-background {
    box-shadow: inset`@`code-padding 0 0 0`@`activeline-number-bgcolor;
}

That code is working nice for me.

[core-ai-bot]

Comment by lkcampbell
Saturday Apr 26, 2014 at 20:10 GMT

---

@TomMalbran if we take into account inline editors though, it seems like we need more changes than this. There is the color for active main editor, inactive main editor, active inline editor, and inactive inline editor.

[core-ai-bot]

Comment by TomMalbran
Saturday Apr 26, 2014 at 21:07 GMT

---

You are right. But we might be able to do all of that in just CSS. The alternative would be to just have an empty gutter and not need to deal with any CSS. I am not sure which one is easier at this point.

[core-ai-bot]

Comment by lkcampbell
Saturday Apr 26, 2014 at 21:16 GMT

---

Sublime Text behaves the way you are suggesting as well. I will play around with it some more. I will make sure that the color of the padding is consistent with the line numbers shown or hidden.

[core-ai-bot]

Comment by lkcampbell
Sunday Apr 27, 2014 at 17:19 GMT

---

@TomMalbran, code review changes committed.

[core-ai-bot]

Comment by lkcampbell
Sunday Apr 27, 2014 at 17:39 GMT

---

Hold on, one more fix I have to do.

[core-ai-bot]

Comment by lkcampbell
Sunday Apr 27, 2014 at 20:58 GMT

---

@TomMalbran, okay, that covers all of the permutations. I'm not sure it's the best way to do the styles, the inline editor style classes are a bit confusing to me. Let me know if you think there is a cleaner way to do this since you have some experience with these styles.

[core-ai-bot]

Comment by lkcampbell
Monday Apr 28, 2014 at 14:57 GMT

---

@TomMalbran, Code Review changes committed.