Correct fix for gutter padding issue in #3211 #3365

Merged
merged 1 commit into from Apr 8, 2013

Conversation

Projects
None yet
3 participants
Member

njx commented Apr 5, 2013

The original fix in #3212 (overriding a style change made in CodeMirror) was a temporary measure until we figured out the real problem. It turns out that the issue is that we were setting vertical padding on both .CodeMirror-lines and .CodeMirror-gutters (since we want a little space at the top of the code area). I think the gutter padding used to be necessary in v2 because the line numbers were actually separate, but now that the line numbers are laid out as part of the lines themselves, it seems like we're only supposed to have it on .CodeMirror-lines. (I'm confirming this with Marijn, but it seems correct just looking at how the DOM is structured now.)

(Also, it was probably wrong for this padding to be on .CodeMirror-gutters to begin with--it was probably on .CodeMirror-gutter in v2, but it was in a block of styles that ended up needing to be on .CodeMirror-gutters instead, so it just got moved along with them.)

The line numbers still seem to line up properly, and the original bug no longer reproduces for me with this change.

jasonsanjose was assigned Apr 5, 2013

Member

njx commented Apr 5, 2013

@jasonsanjose - would you mind looking at this one?

Member

njx commented Apr 5, 2013

Tagging #3211

redmunds was assigned Apr 8, 2013

Member

njx commented Apr 8, 2013

Reassigned to @redmunds

Contributor

redmunds commented Apr 8, 2013

Looks good. Merging.

@redmunds redmunds added a commit that referenced this pull request Apr 8, 2013

@redmunds redmunds Merge pull request #3365 from adobe/nj/issue-3211
Correct fix for gutter padding issue in #3211
b232ea1

@redmunds redmunds merged commit b232ea1 into master Apr 8, 2013

1 check passed

default The Travis build passed
Details

redmunds deleted the nj/issue-3211 branch Apr 8, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment