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

Fix #5062: Update notification is no longer modal #5085

Merged
RaymondLim merged 4 commits into from
Sep 18, 2013
Merged

Fix #5062: Update notification is no longer modal #5085

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
f4101ce
Fixes the modal backdrop
TomMalbran Sep 6, 2013
ae9136d
Moved the check for updates on start up to be done after the app is r…
TomMalbran Sep 10, 2013
ce3a61d
Moved the app ready block to the previous location
TomMalbran Sep 18, 2013
b26d101
Removed unused dependency
TomMalbran Sep 18, 2013
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
1 change: 0 additions & 1 deletion src/brackets.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,6 @@ define(function (require, exports, module) {
// check once a day, plus 2 minutes,
// as the check will skip if the last check was not -24h ago
window.setInterval(UpdateNotification.checkForUpdate, 86520000);
UpdateNotification.checkForUpdate();
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/styles/brackets_patterns_override.less
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -451,7 +451,7 @@
.modal-backdrop {
opacity: 0;
}
.modal-backdrop:last-child {
.last-backdrop {
/* Only show the last modal backdrop */
opacity: 0.5;
}
Expand Down
12 changes: 9 additions & 3 deletions src/utils/UpdateNotification.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,12 @@ define(function (require, exports, module) {

var Dialogs = require("widgets/Dialogs"),
DefaultDialogs = require("widgets/DefaultDialogs"),
NativeApp = require("utils/NativeApp"),
PreferencesManager = require("preferences/PreferencesManager"),
Strings = require("strings"),
StringUtils = require("utils/StringUtils"),
AppInit = require("utils/AppInit"),
Global = require("utils/Global"),
NativeApp = require("utils/NativeApp"),
StringUtils = require("utils/StringUtils"),
Strings = require("strings"),
UpdateDialogTemplate = require("text!htmlContent/update-dialog.html"),
UpdateListTemplate = require("text!htmlContent/update-list.html");

Expand Down Expand Up @@ -330,6 +331,11 @@ define(function (require, exports, module) {
// Append locale to version info URL
_versionInfoURL = brackets.config.update_info_url + brackets.getLocale() + ".json";

// Check for updates on App Ready
AppInit.appReady(function () {
checkForUpdate();
Copy link
Copy Markdown
Contributor

@RaymondLim RaymondLim Sep 17, 2013

Choose a reason for hiding this comment

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

Now we're checking for update regardless of whether "skipUpdateCheck" is specified in the UrlParams or not. When this call is in the old location, it won't be invoked if "skipUpdateCheck" is specified. Not sure how to test with UrlParams. @gruehle, we need your opinion on this.

Copy link
Copy Markdown
Member

@gruehle gruehle Sep 17, 2013

Choose a reason for hiding this comment

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

The "skipUpdateCheck" is required for test windows to work correctly. Without that test, every test window would check for updated.

The call in the old location also checks "inBrowser", since we don't want to do update notification checks while running in browser.

@TomMalbran - is there a reason why this check was moved here? If not, it would be best to keep it in brackets.js.

Copy link
Copy Markdown
Contributor

@RaymondLim RaymondLim Sep 18, 2013

Choose a reason for hiding this comment

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

Thanks @gruehle. Tom moves it here because the old location is too early to show the update dialog and consequently the dialog is losing focus to the main editor causing issue #5602.

@TomMalbran I think we can keep this call back in brackets.js, but in a different location where we're done with asynchronous loading of the editor. That is, moves the entire if block that contains this call to the line above PerfUtils.addMeasurement("Application Startup");.

Copy link
Copy Markdown
Contributor Author

@TomMalbran TomMalbran Sep 18, 2013

Choose a reason for hiding this comment

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

@RaymondLim I was thinking of just moving this block to the old location. I will test if that works, but it might make sense to do it in that way. If it doesn't work i'll test that.

Note: It seems to work fine.

});

// Define public API
exports.checkForUpdate = checkForUpdate;
});
9 changes: 7 additions & 2 deletions src/widgets/Dialogs.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -252,6 +252,7 @@ define(function (require, exports, module) {

// Remove the dialog instance from the DOM.
$dlg.remove();
$(".modal-backdrop:last").addClass("last-backdrop");

// Remove our global keydown handler.
KeyBindingManager.removeGlobalKeydownHook(keydownHook);
Expand All @@ -273,7 +274,9 @@ define(function (require, exports, module) {
_dismissDialog($dlg, $(this).attr("data-button-id"));
});
}


$(".last-backdrop").removeClass("last-backdrop");

// Run the dialog
$dlg
.modal({
Expand All @@ -283,7 +286,9 @@ define(function (require, exports, module) {
})
// Updates the z-index of the modal dialog and the backdrop
.css("z-index", zIndex + 1)
.next().css("z-index", zIndex);
.next()
.css("z-index", zIndex)
.addClass("last-backdrop");

zIndex += 2;

Expand Down
⚡