Skip to content
This repository has been archived by the owner on Sep 6, 2021. It is now read-only.

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

Merged
merged 9 commits into from
Apr 28, 2014
Merged

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

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
bd3cf12
Add left padding to CodeMirror when line numbers are hidden.
lkcampbell Apr 25, 2014
7d5a8e9
Fix text selection problem
lkcampbell Apr 25, 2014
0693ffb
Assign the show-line-padding class to editor holder by default
lkcampbell Apr 25, 2014
440b743
Fix initilization of editor holder line padding
lkcampbell Apr 25, 2014
970b2f9
Create Editor._toggleLinePadding() and small code clean up
lkcampbell Apr 26, 2014
06ac0a5
Add darker background color to padding when line numbers are hidden
lkcampbell Apr 27, 2014
ee10357
Fix inline editor background styles
lkcampbell Apr 27, 2014
dcffe66
Small style clean up
lkcampbell Apr 27, 2014
129bbd6
Change .inline-text-editor styles to nested .CodeMirror styles
lkcampbell Apr 28, 2014
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions src/editor/Editor.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2033,6 +2033,10 @@ define(function (require, exports, module) {
} else if (prefName === SCROLL_PAST_END && this._visibleRange) {
// Do not apply this option to inline editors
return;
} else if (prefName === SHOW_LINE_NUMBERS) {
Editor._toggleLinePadding(!newValue);
this._codeMirror.setOption(cmOptions[SHOW_LINE_NUMBERS], newValue);
this.refreshAll();
} else {
this._codeMirror.setOption(cmOptions[prefName], newValue);
}
Expand Down Expand Up @@ -2204,6 +2208,16 @@ define(function (require, exports, module) {
return PreferencesManager.get(WORD_WRAP, fullPath);
};

/**
* @private
* Toggles the left padding of all code editors. Used to provide more
* space between the code text and the left edge of the editor when
* line numbers are hidden.
* @param {boolean} showLinePadding
*/
Editor._toggleLinePadding = function (showLinePadding) {
$("#editor-holder").toggleClass("show-line-padding", showLinePadding);
};

// Set up listeners for preference changes
editorOptions.forEach(function (prefName) {
Expand Down
4 changes: 4 additions & 0 deletions src/editor/EditorOptionHandlers.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,10 @@ define(function (require, exports, module) {
_.each(_optionMapping, function (commandName, prefName) {
CommandManager.get(commandName).setChecked(PreferencesManager.get(prefName));
});

if (!Editor.getShowLineNumbers()) {
Editor._toggleLinePadding(true);
}
}

CommandManager.register(Strings.CMD_TOGGLE_LINE_NUMBERS, Commands.TOGGLE_LINE_NUMBERS, _getToggler(SHOW_LINE_NUMBERS));
Expand Down
19 changes: 19 additions & 0 deletions src/styles/brackets_codemirror_override.less
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,10 @@
padding: 0 @code-padding 0 0;
}

.show-line-padding .CodeMirror pre {
padding-left: @code-padding;
}

.CodeMirror-scroll {
background-color: @background-color-3;
}
Expand All @@ -45,9 +49,15 @@
.CodeMirror-activeline-background {
background: transparent;
}

.CodeMirror-focused .CodeMirror-activeline-background {
background: @activeline-bgcolor;
}

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

.CodeMirror-focused .CodeMirror-activeline {
& > div, .CodeMirror-gutter-elt {
height: 100%;
Expand Down Expand Up @@ -228,6 +238,7 @@
.CodeMirror .CodeMirror-activeline-background {
background: transparent;
}

.CodeMirror .CodeMirror-activeline .CodeMirror-gutter-elt {
background: transparent;
color: @accent-comment;
Expand All @@ -247,6 +258,14 @@
.CodeMirror-matchingtag { background: @matching-bracket; }
}

.show-line-padding .inline-text-editor .CodeMirror-activeline-background {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is working great, but we never used .inline-text-editor, so just to be more consistent with the other classes, we could move this new 2 styles to line 258 and change them to:

    .show-line-padding & .CodeMirror .CodeMirror-activeline-background {
        box-shadow: none;
    }
    .show-line-padding & .CodeMirror-focused .CodeMirror-activeline-background {
        box-shadow: inset @code-padding 0 0 0 @tc-gray-panel-top-bar;
    }

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@TomMalbran, I agree that for consistency sake, your suggestion is correct. I will update soon.

Just to express my opinion, though, I think using .inline-text-editor in the rules is much easier to read and understand than the nested, double .CodeMirror hack we are currently using.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't knew about that class until now. It seems nice but we need to be sure that everything works fine after using it. Maybe you can open a new issue for that, or discuss it in the forums.

box-shadow: inset @code-padding 0 0 0 transparent;
}

.show-line-padding .inline-text-editor .CodeMirror-focused .CodeMirror-activeline-background {
box-shadow: inset @code-padding 0 0 0 @tc-gray-panel-top-bar;
}

/*
* Temporarily override bold and italic syntax highlighting until
* SourceCodePro supports them in a fixed pitch
Expand Down