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

Fix for issue #1561 #1707

Merged
merged 7 commits into from
Oct 3, 2012
Merged

Fix for issue #1561 #1707

Changes from 2 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
f82a22a
fix for issue 1561
Sep 24, 2012
6759e87
Fix for Issue 1561 - added eventHandler function for statusChange
Sep 26, 2012
9539c42
Merge remote-tracking branch 'upstream/master' into issue1561
Sep 29, 2012
03bb814
Changes to add checkmark only for STATUS_ACTIVE
Sep 29, 2012
563117b
Unnecessary whitespace cleanup
Sep 29, 2012
2d4d641
Merge remote-tracking branch 'upstream/master' into issue1561
Oct 3, 2012
7306f52
Refined condition for adding checkmark
Oct 3, 2012
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
20 changes: 17 additions & 3 deletions src/LiveDevelopment/main.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,6 @@ define(function main(require, exports, module) {
function _handleGoLiveCommand() {
if (LiveDevelopment.status >= LiveDevelopment.STATUS_CONNECTING) {
LiveDevelopment.close();
// TODO Ty: when checkmark support lands, remove checkmark
} else {
if (!params.get("skipLiveDevelopmentInfo") && !prefs.getValue("afterFirstLaunch")) {
prefs.setValue("afterFirstLaunch", "true");
Expand All @@ -132,7 +131,6 @@ define(function main(require, exports, module) {
} else {
LiveDevelopment.open();
}
// TODO Ty: when checkmark support lands, add checkmark
}
}

Expand All @@ -155,6 +153,20 @@ define(function main(require, exports, module) {
// Initialize tooltip for 'not connected' state
_setLabel(_$btnGoLive, null, _statusStyle[1], _statusTooltip[1]);
}

/** maintains state of the menu item */
function _setupGoLiveMenu() {
$(LiveDevelopment).on("statusChange", function statusChange(event, status) {
//update the checkmark next to 'Live Preview' menu item
//add checkmark when status is STATUS_CONNECTING
Copy link

Choose a reason for hiding this comment

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

Minor nit: we generally put one space after // and capitalize comments.

if (status === LiveDevelopment.STATUS_CONNECTING) {
Copy link

Choose a reason for hiding this comment

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

I think it would be better to only show the checkmark when we actually finish establishing the connection (STATUS_ACTIVE).

CommandManager.get(Commands.FILE_LIVE_FILE_PREVIEW).setChecked(true);
} else if (status < LiveDevelopment.STATUS_CONNECTING) {
//remove checkmark if status is STATUS_INACTIVE or STATUS_ERROR
CommandManager.get(Commands.FILE_LIVE_FILE_PREVIEW).setChecked(false);
Copy link

Choose a reason for hiding this comment

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

If we only show the checkmark when the status is STATUS_ACTIVE, then you could just remove it in all other cases.

}
});
}

/** Create the menu item "Highlight" */
function _setupHighlightButton() {
Expand Down Expand Up @@ -191,6 +203,8 @@ define(function main(require, exports, module) {
LiveDevelopment.init(config);
_loadStyles();
_setupGoLiveButton();
_setupGoLiveMenu();

/* _setupHighlightButton(); FUTURE - Highlight button */
if (config.debug) {
_setupDebugHelpers();
Expand All @@ -209,9 +223,9 @@ define(function main(require, exports, module) {
}
});
}

}
window.setTimeout(init);

CommandManager.register(Strings.CMD_LIVE_FILE_PREVIEW, Commands.FILE_LIVE_FILE_PREVIEW, _handleGoLiveCommand);

// Export public functions
Expand Down