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

Commit

Permalink
Migrate "fontSizeAdjustment" to new view states.
Browse files Browse the repository at this point in the history
  • Loading branch information
RaymondLim committed Mar 14, 2014
1 parent ebb6b7a commit 8d0abbd
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 3 deletions.
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
21 changes: 18 additions & 3 deletions src/view/ViewCommandHandlers.js
Original file line number Diff line number Diff line change
Expand Up @@ -323,6 +323,23 @@ 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) {
var fontSize = 12 + value,
newRule = { "fontSizeStyle": fontSize + "px",
"lineHeightStyle": Math.ceil(fontSize * LINE_HEIGHT) + "px" };

return newRule;
}

// Register command handlers
CommandManager.register(Strings.CMD_INCREASE_FONT_SIZE, Commands.VIEW_INCREASE_FONT_SIZE, _handleIncreaseFontSize);
Expand All @@ -331,9 +348,7 @@ 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);
Expand Down

4 comments on commit 8d0abbd

@lkcampbell
Copy link
Contributor

Choose a reason for hiding this comment

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

@RaymondLim and @TomMalbran, I just selected this commit a bit arbitrarily because don't know exactly when this problem happened. The problem is the fontSizeChange event no longer fires off when I select Restore Font Size. It works fine for Increase Font Size and Decrease Font Size, but it appears to have been removed for Restore Font Size. It's breaking my Column Ruler extension.

@TomMalbran
Copy link
Contributor

Choose a reason for hiding this comment

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

You are right. When restoring the font size, we no longer call the adjust font size function which triggers the event. Should be easy to fix by addling trigger on the handle restore font code.

@RaymondLim
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 Do you want to submit a pr adding the trigger for restore font size function?

@lkcampbell
Copy link
Contributor

Choose a reason for hiding this comment

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

@RaymondLim and @TomMalbran, I got it: #7443

Please sign in to comment.