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

Commit

Permalink
Fix LiveDevelopment initialization.
Browse files Browse the repository at this point in the history
PreferencesManager.on("change") was executing before AppInit.appReady().
This was causing various inconsistencies, including setting up the UI before
LiveDevelopment is actually initialized. Handling of pref change is now moved
to AppInit.appReady and the implementation is set only after the modules are
actually initialized.

Fix #10239: toggle menu to switch livedev impls.
    - Ensure correct status changes when switching implementations.
    - Disable 'Project Settings' when multi-browser is on.

Also:
    - Minor improvements to how the pref is defined.
    - Close LiveDevelopment explicitly after each test.
    - Minor refactor/doc changes in LiveDevelopment/main.js
    - Minor stylistic and documentation changes.
  • Loading branch information
busykai committed Jan 15, 2015
1 parent 93be1de commit eef9c68
Show file tree
Hide file tree
Showing 7 changed files with 82 additions and 33 deletions.
7 changes: 5 additions & 2 deletions src/LiveDevelopment/LiveDevMultiBrowser.js
Original file line number Diff line number Diff line change
Expand Up @@ -467,10 +467,13 @@ define(function (require, exports, module) {
}

/**
* Close all active connections
* Closes all active connections.
* Returns a resolved promise for API compatibility.
* @return {$.Promise} A resolved promise
*/
function close() {
return _close(true);
_close(true);
return new $.Deferred().resolve().promise();
}

/**
Expand Down
1 change: 1 addition & 0 deletions src/LiveDevelopment/LiveDevelopment.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@
* 2: Loading agents
* 3: Active
* 4: Out of sync
* 5: Sync error
*
* The reason codes are:
* - null (Unknown reason)
Expand Down
100 changes: 71 additions & 29 deletions src/LiveDevelopment/main.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ define(function main(require, exports, module) {
Commands = require("command/Commands"),
AppInit = require("utils/AppInit"),
LiveDevelopment = require("LiveDevelopment/LiveDevelopment"),
MultiBrowserLiveDev = require("LiveDevelopment/LiveDevMultiBrowser"),
Inspector = require("LiveDevelopment/Inspector/Inspector"),
CommandManager = require("command/CommandManager"),
PreferencesManager = require("preferences/PreferencesManager"),
Expand All @@ -50,9 +51,6 @@ define(function main(require, exports, module) {
ExtensionUtils = require("utils/ExtensionUtils"),
StringUtils = require("utils/StringUtils");

// expermiental multi-browser implementation
var MultiBrowserLiveDev = require("LiveDevelopment/LiveDevMultiBrowser");

var params = new UrlParams();
var config = {
experimental: false, // enable experimental features
Expand All @@ -75,7 +73,41 @@ define(function main(require, exports, module) {

// current selected implementation (LiveDevelopment | LiveDevMultiBrowser)
var LiveDevImpl;

// "livedev.multibrowser" preference
var PREF_MULTIBROWSER = "multibrowser";
var prefs = PreferencesManager.getExtensionPrefs("livedev");
var multiBrowserPref = prefs.definePreference(PREF_MULTIBROWSER, "boolean", false);

/** Toggles or sets the preference **/
function _togglePref(key, value) {
var val,
oldPref = !!prefs.get(key);

if (value === undefined) {
val = !oldPref;
} else {
val = !!value;
}

// update menu
if (val !== oldPref) {
prefs.set(key, val);
}

return val;
}

/* Toggles or sets the "livedev.multibrowser" preference */
function _toggleLivePreviewMultiBrowser(value) {
var val = _togglePref(PREF_MULTIBROWSER, value);

CommandManager.get(Commands.TOGGLE_LIVE_PREVIEW_MB_MODE).setChecked(val);
// Issue #10217: multi-browser does not support user server, so disable
// the setting if it is enabled.
CommandManager.get(Commands.FILE_PROJECT_SETTINGS).setEnabled(!val);
}

/** Load Live Development LESS Style */
function _loadStyles() {
var lessText = require("text!LiveDevelopment/main.less"),
Expand Down Expand Up @@ -215,10 +247,11 @@ define(function main(require, exports, module) {
PreferencesManager.setViewState("livedev.highlight", config.highlight);
}

// sets the MultiBrowserLiveDev implementation if multibrowser = true,
// keeps default LiveDevelopment implementation based on CDT in other case.
// since UI status are slightly different btw implementations, it also set
// the corresponding style values.
/**
* Sets the MultiBrowserLiveDev implementation if multibrowser is truthy,
* keeps default LiveDevelopment implementation based on CDT otherwise.
* It also resets the listeners and UI elements.
*/
function _setImplementation(multibrowser) {
if (multibrowser) {
// set implemenation
Expand Down Expand Up @@ -246,6 +279,11 @@ define(function main(require, exports, module) {
{ tooltip: Strings.LIVE_DEV_STATUS_TIP_SYNC_ERROR, style: "sync-error" }
];
}
// setup status changes listeners for new implementation
_setupGoLiveButton();
_setupGoLiveMenu();
// toggle the menu
_toggleLivePreviewMultiBrowser(multibrowser);
}

/** Setup window references to useful LiveDevelopment modules */
Expand All @@ -261,7 +299,7 @@ define(function main(require, exports, module) {
LiveDevelopment.reload();
}
}

/** Initialize LiveDevelopment */
AppInit.appReady(function () {
params.parse();
Expand All @@ -274,13 +312,12 @@ define(function main(require, exports, module) {
// It has to be initiated at this point in case of dynamically switching
// by changing the preference value.
MultiBrowserLiveDev.init(config);

_loadStyles();
_setupGoLiveButton();
_setupGoLiveMenu();

_loadStyles();
_updateHighlightCheckmark();

_setImplementation(prefs.get(PREF_MULTIBROWSER));

if (config.debug) {
_setupDebugHelpers();
}
Expand All @@ -299,6 +336,25 @@ define(function main(require, exports, module) {
LiveDevelopment.redrawHighlight();
}
});

multiBrowserPref
.on("change", function () {
// Stop the current session if it is open and set implementation based on
// the pref value. We could start the new implementation immediately, but
// since the current document is potentially a user preferences file, Live
// Preview will not locate the html file to serve.
if (LiveDevImpl && LiveDevImpl.status >= LiveDevImpl.STATUS_ACTIVE) {
LiveDevImpl.close()
.done(function () {
// status changes will now be listened by the new implementation
LiveDevImpl.off("statusChange");
_setImplementation(prefs.get(PREF_MULTIBROWSER));
});
} else {
_setImplementation(prefs.get(PREF_MULTIBROWSER));
}
});

});

// init prefs
Expand All @@ -312,29 +368,15 @@ define(function main(require, exports, module) {
"highlight": "user livedev.highlight",
"afterFirstLaunch": "user livedev.afterFirstLaunch"
}, true);

PreferencesManager.definePreference("livedev.multibrowser", "boolean", false)
.on("change", function () {
// stop the current session if it is open and set implementation based on
// the pref value. It could be automaticallty restarted but, since the preference file,
// is the document open in the editor, it will no launch a live document.
if (LiveDevImpl && LiveDevImpl.status >= LiveDevImpl.STATUS_ACTIVE) {
LiveDevImpl.close();
// status changes will now be listen from the new implementation
LiveDevImpl.off('statusChange');
}
_setImplementation(PreferencesManager.get('livedev.multibrowser'));
// setup status changes listeners for new implementation
_setupGoLiveButton();
_setupGoLiveMenu();
});


config.highlight = PreferencesManager.getViewState("livedev.highlight");

// init commands
CommandManager.register(Strings.CMD_LIVE_FILE_PREVIEW, Commands.FILE_LIVE_FILE_PREVIEW, _handleGoLiveCommand);
CommandManager.register(Strings.CMD_LIVE_HIGHLIGHT, Commands.FILE_LIVE_HIGHLIGHT, _handlePreviewHighlightCommand);
CommandManager.register(Strings.CMD_RELOAD_LIVE_PREVIEW, Commands.CMD_RELOAD_LIVE_PREVIEW, _handleReloadLivePreviewCommand);
CommandManager.register(Strings.CMD_TOGGLE_LIVE_PREVIEW_MB_MODE, Commands.TOGGLE_LIVE_PREVIEW_MB_MODE, _toggleLivePreviewMultiBrowser);

CommandManager.get(Commands.FILE_LIVE_HIGHLIGHT).setEnabled(false);

// Export public functions
Expand Down
1 change: 1 addition & 0 deletions src/command/Commands.js
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ define(function (require, exports, module) {
exports.FILE_CLOSE_LIST = "file.close_list"; // DocumentCommandHandlers.js handleFileCloseList()
exports.FILE_OPEN_DROPPED_FILES = "file.openDroppedFiles"; // DragAndDrop.js openDroppedFiles()
exports.FILE_LIVE_FILE_PREVIEW = "file.liveFilePreview"; // LiveDevelopment/main.js _handleGoLiveCommand()
exports.TOGGLE_LIVE_PREVIEW_MB_MODE = "file.toggleLivePreviewMB"; // LiveDevelopment/main.js _toggleLivePreviewMultiBrowser()
exports.CMD_RELOAD_LIVE_PREVIEW = "file.reloadLivePreview"; // LiveDevelopment/main.js _handleReloadLivePreviewCommand()
exports.FILE_LIVE_HIGHLIGHT = "file.previewHighlight"; // LiveDevelopment/main.js _handlePreviewHighlightCommand()
exports.FILE_PROJECT_SETTINGS = "file.projectSettings"; // ProjectManager.js _projectSettings()
Expand Down
1 change: 1 addition & 0 deletions src/command/DefaultMenus.js
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ define(function (require, exports, module) {
menu.addMenuItem(Commands.FILE_SAVE_AS);
menu.addMenuDivider();
menu.addMenuItem(Commands.FILE_LIVE_FILE_PREVIEW);
menu.addMenuItem(Commands.TOGGLE_LIVE_PREVIEW_MB_MODE);
menu.addMenuItem(Commands.FILE_PROJECT_SETTINGS);
menu.addMenuDivider();
menu.addMenuItem(Commands.FILE_EXTENSION_MANAGER);
Expand Down
1 change: 1 addition & 0 deletions src/nls/root/strings.js
Original file line number Diff line number Diff line change
Expand Up @@ -317,6 +317,7 @@ define({
"CMD_FILE_SAVE_ALL" : "Save All",
"CMD_FILE_SAVE_AS" : "Save As\u2026",
"CMD_LIVE_FILE_PREVIEW" : "Live Preview",
"CMD_TOGGLE_LIVE_PREVIEW_MB_MODE" : "Enable Experimental Live Preview",
"CMD_RELOAD_LIVE_PREVIEW" : "Force Reload Live Preview",
"CMD_PROJECT_SETTINGS" : "Project Settings\u2026",
"CMD_FILE_RENAME" : "Rename",
Expand Down
4 changes: 2 additions & 2 deletions test/spec/LiveDevelopmentMultiBrowser-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ define(function (require, exports, module) {

var SpecRunnerUtils = require("spec/SpecRunnerUtils");

describe("MultiBrowser (experimental) - LiveDevelopment", function () {
describe("MultiBrowser (experimental)", function () {

this.category = "livepreview";

Expand Down Expand Up @@ -66,6 +66,7 @@ define(function (require, exports, module) {
});

afterEach(function () {
LiveDevelopment.close();
SpecRunnerUtils.closeTestWindow();
testWindow = null;
brackets = null;
Expand All @@ -88,7 +89,6 @@ define(function (require, exports, module) {

describe("Init Session", function () {


it("should establish a browser connection for an opened html file", function () {
//open a file
runs(function () {
Expand Down

0 comments on commit eef9c68

Please sign in to comment.