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

Set the font size before opening the documents #7185

Merged
merged 7 commits into from
Mar 14, 2014
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
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
3 changes: 2 additions & 1 deletion src/brackets.js
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ define(function (require, exports, module) {
ColorUtils = require("utils/ColorUtils"),
CodeInspection = require("language/CodeInspection"),
NativeApp = require("utils/NativeApp"),
ViewCommandHandlers = require("view/ViewCommandHandlers"),
_ = require("thirdparty/lodash");

// Load modules that self-register and just need to get included in the main project
Expand All @@ -107,7 +108,6 @@ define(function (require, exports, module) {
require("editor/EditorStatusBar");
require("editor/EditorCommandHandlers");
require("editor/EditorOptionHandlers");
require("view/ViewCommandHandlers");
require("help/HelpCommandHandlers");
require("search/FindInFiles");
require("search/FindReplace");
Expand Down Expand Up @@ -220,6 +220,7 @@ define(function (require, exports, module) {
// Load the initial project after extensions have loaded
extensionLoaderPromise.always(function () {
// Finish UI initialization
ViewCommandHandlers.restoreFontSize();
var initialProjectPath = ProjectManager.getInitialProjectPath();
ProjectManager.openProject(initialProjectPath).always(function () {
_initTest();
Expand Down
12 changes: 12 additions & 0 deletions src/preferences/PreferenceStorage.js
Original file line number Diff line number Diff line change
Expand Up @@ -218,6 +218,13 @@ define(function (require, exports, module) {
var rule = rules[key];
if (!rule && prefCheckCallback) {
rule = prefCheckCallback(key);
} else if (prefCheckCallback) {
// Check whether we have a new preference key-value pair
// for an old preference.
var newRule = prefCheckCallback(key, prefs[key]);
if (newRule) {
rule = _.cloneDeep(newRule);
}
}
if (!rule) {
console.warn("Preferences conversion for ", self._clientID, " has no rule for", key);
Expand All @@ -238,6 +245,11 @@ define(function (require, exports, module) {
manager.set(newKey, prefs[key], options);
convertedKeys.push(key);
}
} else if (_.isObject(rule)) {
Object.keys(rule).forEach(function (ruleKey) {
manager.set(ruleKey, rule[ruleKey]);
});
convertedKeys.push(key);
} else {
complete = false;
}
Expand Down
4 changes: 1 addition & 3 deletions src/styles/brackets_theme_default.less
Original file line number Diff line number Diff line change
Expand Up @@ -144,10 +144,8 @@
*/
.code-font() {
color: @content-color;
// line-height must be specified in px not em because the code font and line number font sizes are different.
// Sizing via em will cause the code and line numbers to misalign
line-height: 15px;
font-size: 12px;
line-height: 1.25;
font-family: "SourceCodePro-Medium", "MS ゴシック", "MS Gothic", monospace;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This comment is no longer valid, since the line numbers are now in the same div as the rest of the line. Also if we wanted to decrease the font-size value of the line numbers, we can easily increase the line-height value.

}

Expand Down
161 changes: 81 additions & 80 deletions src/view/ViewCommandHandlers.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,6 @@
* Increase Font Size, Decrease Font Size, or Restore Font Size commands.
* The 2nd arg to the listener is the amount of the change. The 3rd arg
* is a string containing the new font size after applying the change.
* The 4th arg is a string containing the new line height after applying
* the change.
*/

define(function (require, exports, module) {
Expand Down Expand Up @@ -78,12 +76,6 @@ define(function (require, exports, module) {
*/
var LINE_HEIGHT = 1.25;

/**
* @private
* @type {boolean}
*/
var _fontSizePrefsLoaded = false;


/**
* @private
Expand All @@ -93,41 +85,41 @@ define(function (require, exports, module) {
$("#" + DYNAMIC_FONT_STYLE_ID).remove();
}

/**
* @private
* Add the styles used to update the font size
* @param {string} fontSizeStyle A string with the font size and the size unit
*/
function _addDynamicFontSize(fontSizeStyle) {
var style = $("<style type='text/css'></style>").attr("id", DYNAMIC_FONT_STYLE_ID);
style.html(".CodeMirror { font-size: " + fontSizeStyle + " !important; }");
$("head").append(style);
}

/**
* @private
* Sets the font size and restores the scroll position as best as possible.
* @param {string} fontSizeStyle A string with the font size and the size unit
* @param {string} lineHeightStyle A string with the line height and a the size unit
* @param {string=} fontSizeStyle A string with the font size and the size unit
*/
function _setSizeAndRestoreScroll(fontSizeStyle, lineHeightStyle) {
function _setSizeAndRestoreScroll(fontSizeStyle) {
var editor = EditorManager.getCurrentFullEditor(),
oldWidth = editor._codeMirror.defaultCharWidth(),
oldHeight = editor.getTextHeight(),
scrollPos = editor.getScrollPos();
scrollPos = editor.getScrollPos(),
line = editor._codeMirror.lineAtHeight(scrollPos.y, "local");
Copy link
Contributor

Choose a reason for hiding this comment

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

cmv4 is already landed in master and I wonder lineAtHeight and heightAtLine are still the same in cmv4. Can you double check or can you merge master to your pull request?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure will try, hopefully it should behave the same way.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually I made this PR after cmv4 landed.

Copy link
Contributor

Choose a reason for hiding this comment

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

Really, but I notice that CM submodule is different.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I can use multiple cursors in this branch.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok.


// It's necessary to inject a new rule to address all editors.
_removeDynamicFontSize();
var style = $("<style type='text/css'></style>").attr("id", DYNAMIC_FONT_STYLE_ID);
style.html(".CodeMirror {" +
"font-size: " + fontSizeStyle + " !important;" +
"line-height: " + lineHeightStyle + " !important;}");
$("head").append(style);

if (fontSizeStyle) {
_addDynamicFontSize(fontSizeStyle);
}
editor.refreshAll();

// Scroll the document back to its original position, but not on the first load since the position
// was saved with the new height and already been restored.
if (_fontSizePrefsLoaded) {
// Calculate the new scroll based on the old font sizes and scroll position
var newWidth = editor._codeMirror.defaultCharWidth(),
newHeight = editor.getTextHeight(),
deltaX = scrollPos.x / oldWidth,
deltaY = scrollPos.y / oldHeight,
scrollPosX = scrollPos.x + Math.round(deltaX * (newWidth - oldWidth)),
scrollPosY = scrollPos.y + Math.round(deltaY * (newHeight - oldHeight));

editor.setScrollPos(scrollPosX, scrollPosY);
}
// Calculate the new scroll based on the old font sizes and scroll position
var newWidth = editor._codeMirror.defaultCharWidth(),
deltaX = scrollPos.x / oldWidth,
scrollPosX = scrollPos.x + Math.round(deltaX * (newWidth - oldWidth)),
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 not related to the two issues you're fixing, but I wonder this calculation is always correct for a long line that can wrap the horizontal scroll position to a new line with Word Wrap on.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

if Word Wrap is on, you can't scroll horizontally, so even trying to make ti scroll horizontally, it will always stay at 0.

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, you're right!

scrollPosY = editor._codeMirror.heightAtLine(line, "local");

editor.setScrollPos(scrollPosX, scrollPosY);
}

/**
Expand All @@ -137,68 +129,48 @@ define(function (require, exports, module) {
* @return {boolean} true if adjustment occurred, false if it did not occur
*/
function _adjustFontSize(adjustment) {
var fsStyle = $(".CodeMirror").css("font-size");
var lhStyle = $(".CodeMirror").css("line-height");

var validFont = /^[\d\.]+(px|em)$/;
var fsStyle = $(".CodeMirror").css("font-size"),
validFont = /^[\d\.]+(px|em)$/;

// Make sure the font size and line height are expressed in terms
// we can handle (px or em). If not, simply bail.
if (fsStyle.search(validFont) === -1 || lhStyle.search(validFont) === -1) {
// Make sure that the font size is expressed in terms we can handle (px or em). If not, simply bail.
if (fsStyle.search(validFont) === -1) {
return false;
}

// Guaranteed to work by the validation above.
var fsUnits = fsStyle.substring(fsStyle.length - 2, fsStyle.length);
var lhUnits = lhStyle.substring(lhStyle.length - 2, lhStyle.length);
var delta = (fsUnits === "px") ? 1 : 0.1;

var fsOld = parseFloat(fsStyle.substring(0, fsStyle.length - 2));
var lhOld = parseFloat(lhStyle.substring(0, lhStyle.length - 2));

var fsNew = fsOld + (delta * adjustment);
var lhNew = lhOld;
if (fsUnits === lhUnits) {
lhNew = fsNew * LINE_HEIGHT;
if (lhUnits === "px") {
// Use integer px value to avoid rounding differences
lhNew = Math.ceil(lhNew);
}
}

var fsStr = fsNew + fsUnits;
var lhStr = lhNew + lhUnits;
var fsUnits = fsStyle.substring(fsStyle.length - 2, fsStyle.length),
delta = fsUnits === "px" ? 1 : 0.1,
fsOld = parseFloat(fsStyle.substring(0, fsStyle.length - 2)),
fsNew = fsOld + (delta * adjustment),
fsStr = fsNew + fsUnits;

// Don't let the font size get too small or too large. The minimum font size is 1px or 0.1em
// and the maximum font size is 72px or 7.2em depending on the unit used
if (fsNew < MIN_FONT_SIZE * delta || fsNew > MAX_FONT_SIZE * delta) {
return false;
}

_setSizeAndRestoreScroll(fsStr, lhStr);
_setSizeAndRestoreScroll(fsStr);
PreferencesManager.setViewState("fontSizeStyle", fsStr);

$(exports).triggerHandler("fontSizeChange", [adjustment, fsStr, lhStr]);
$(exports).triggerHandler("fontSizeChange", [adjustment, fsStr]);
return true;
}

/** Increases the font size by 1 */
function _handleIncreaseFontSize() {
if (_adjustFontSize(1)) {
PreferencesManager.setViewState("fontSizeAdjustment", PreferencesManager.getViewState("fontSizeAdjustment") + 1);
}
_adjustFontSize(1);
}

/** Decreases the font size by 1 */
function _handleDecreaseFontSize() {
if (_adjustFontSize(-1)) {
PreferencesManager.setViewState("fontSizeAdjustment", PreferencesManager.getViewState("fontSizeAdjustment") - 1);
}
_adjustFontSize(-1);
}

/** Restores the font size to the original size */
function _handleRestoreFontSize() {
_adjustFontSize(-PreferencesManager.getViewState("fontSizeAdjustment"));
PreferencesManager.setViewState("fontSizeAdjustment", 0);
_setSizeAndRestoreScroll();
PreferencesManager.setViewState("fontSizeStyle");
}


Expand All @@ -215,14 +187,6 @@ define(function (require, exports, module) {
CommandManager.get(Commands.VIEW_DECREASE_FONT_SIZE).setEnabled(true);
CommandManager.get(Commands.VIEW_RESTORE_FONT_SIZE).setEnabled(true);
}

// Font Size preferences only need to be loaded one time
if (!_fontSizePrefsLoaded) {
_removeDynamicFontSize();
_adjustFontSize(PreferencesManager.getViewState("fontSizeAdjustment"));
_fontSizePrefsLoaded = true;
}

} else {
// No current document so disable all of the Font Size commands
CommandManager.get(Commands.VIEW_INCREASE_FONT_SIZE).setEnabled(false);
Expand All @@ -231,6 +195,30 @@ define(function (require, exports, module) {
}
}

/**
* Restores the Font Size and Line Height using the saved strings
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: We can take out Line Height here.

*/
function restoreFontSize() {
var fsStyle = PreferencesManager.getViewState("fontSizeStyle"),
fsAdjustment = PreferencesManager.getViewState("fontSizeAdjustment");

if (fsAdjustment) {
// Always remove the old view state even if we also have the new view state.
PreferencesManager.setViewState("fontSizeAdjustment");

if (!fsStyle) {
// Migrate the old view state to the new one.
fsStyle = (12 + fsAdjustment) + "px";
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you define a constant for default font size and replace the two instances of the magic number 12?

PreferencesManager.setViewState("fontSizeStyle", fsStyle);
}
}

if (fsStyle) {
_removeDynamicFontSize();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I left this remove here, even though we don't need it on the initial load, but it is required if this method is ever called after appReady.

Copy link
Contributor

Choose a reason for hiding this comment

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

Perfectly fine to call it here to ensure that we don't miss any.

_addDynamicFontSize(fsStyle);
}
}



/**
Expand Down Expand Up @@ -319,6 +307,19 @@ define(function (require, exports, module) {
_scrollLine(1);
}

/**
* @private
* Convert the old "fontSizeAdjustment" preference to the new view states.
*
* @param {string} key The key of the preference to be examined for migration
* of old preferences. Not used since we only have one in this module.
* @param {string} value The value of "fontSizeAdjustment" preference
* @return {Object} - JSON object for the new view states equivalent to
* the old "fontSizeAdjustment" preference.
*/
function _convertToNewViewStates(key, value) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice job in cleaning. Just one more minor nit since we only have one new view state, can you also rename this to singular _convertToNewViewState and update the callback argument in convertPreferences call below?

return { "fontSizeStyle": (12 + value) + "px" };
}

// Register command handlers
CommandManager.register(Strings.CMD_INCREASE_FONT_SIZE, Commands.VIEW_INCREASE_FONT_SIZE, _handleIncreaseFontSize);
Expand All @@ -327,13 +328,13 @@ define(function (require, exports, module) {
CommandManager.register(Strings.CMD_SCROLL_LINE_UP, Commands.VIEW_SCROLL_LINE_UP, _handleScrollLineUp);
CommandManager.register(Strings.CMD_SCROLL_LINE_DOWN, Commands.VIEW_SCROLL_LINE_DOWN, _handleScrollLineDown);

// Initialize the default font size
PreferencesManager.stateManager.definePreference("fontSizeAdjustment", "number", 0);
PreferencesManager.convertPreferences(module, {"fontSizeAdjustment": "user"}, true);
PreferencesManager.convertPreferences(module, {"fontSizeAdjustment": "user"}, true, _convertToNewViewStates);

// Update UI when opening or closing a document
$(DocumentManager).on("currentDocumentChange", _updateUI);

// Update UI when Brackets finishes loading
AppInit.appReady(_updateUI);

exports.restoreFontSize = restoreFontSize;
});