From e44f1449a2af6f7d3a6239e431495ddd56f48288 Mon Sep 17 00:00:00 2001 From: Marcel Gerber Date: Sun, 27 Apr 2014 18:26:55 +0200 Subject: [PATCH 1/6] Show all contributors in the AboutDialog --- src/brackets.config.json | 2 +- src/config.json | 2 +- src/help/HelpCommandHandlers.js | 57 ++++++++++++++++++---- src/styles/brackets_patterns_override.less | 15 +++--- 4 files changed, 58 insertions(+), 18 deletions(-) diff --git a/src/brackets.config.json b/src/brackets.config.json index e39c6bc14d5..cf0d700a753 100644 --- a/src/brackets.config.json +++ b/src/brackets.config.json @@ -14,7 +14,7 @@ "twitter_url" : "https://twitter.com/brackets", "troubleshoot_url" : "https://github.com/adobe/brackets/wiki/Troubleshooting#wiki-livedev", "twitter_name" : "@brackets", - "contributors_url" : "https://api.github.com/repos/adobe/brackets/contributors?per_page=300", + "contributors_url" : "https://api.github.com/repos/adobe/brackets/contributors", "extension_listing_url" : "", "extension_registry" : "https://s3.amazonaws.com/extend.brackets/registry.json", "extension_url" : "https://s3.amazonaws.com/extend.brackets/{0}/{0}-{1}.zip", diff --git a/src/config.json b/src/config.json index 9f1fc2d7ebd..f88388902fc 100644 --- a/src/config.json +++ b/src/config.json @@ -13,7 +13,7 @@ "twitter_url": "https://twitter.com/brackets", "troubleshoot_url": "https://github.com/adobe/brackets/wiki/Troubleshooting#wiki-livedev", "twitter_name": "@brackets", - "contributors_url": "https://api.github.com/repos/adobe/brackets/contributors?per_page=300", + "contributors_url": "https://api.github.com/repos/adobe/brackets/contributors", "extension_listing_url": "", "extension_registry": "https://s3.amazonaws.com/extend.brackets/registry.json", "extension_url": "https://s3.amazonaws.com/extend.brackets/{0}/{0}-{1}.zip", diff --git a/src/help/HelpCommandHandlers.js b/src/help/HelpCommandHandlers.js index bd5c8a3fd04..9b46ab74d77 100644 --- a/src/help/HelpCommandHandlers.js +++ b/src/help/HelpCommandHandlers.js @@ -78,18 +78,57 @@ define(function (require, exports, module) { // Get containers var $dlg = $(".about-dialog.instance"), $contributors = $dlg.find(".about-contributors"), - $spinner = $dlg.find(".spinner"); + $spinner = $dlg.find(".spinner"), + contributorsUrl = brackets.config.contributors_url, + page, + allContributors = [], + data; + + if (contributorsUrl.indexOf("api.github.com") !== -1) { + page = 1; + } $spinner.addClass("spin"); - // Get all the project contributors and add them to the dialog - $.getJSON(brackets.config.contributors_url).done(function (contributorsInfo) { - + function loadContributorsPage(url, page) { + var contributors; + + if (page) { + url += "?per_page=100&page=" + page; + } + $.ajax({ + url: url, + async: false, + dataType: "json", + cache: false, + complete: function (data) { + contributors = (data && data.responseJSON) || []; + } + }); + return contributors; + } + + // Get all the project contributors + do { + data = loadContributorsPage(contributorsUrl, page); + allContributors = allContributors.concat(data); + if (page) { + page++; + } + } while (page && data.length === 100); + + if (allContributors.length) { // Populate the contributors data - var totalContributors = contributorsInfo.length; - var contributorsCount = 0; + var totalContributors = allContributors.length, + contributorsCount = 0; - $contributors.html(Mustache.render(ContributorsTemplate, contributorsInfo)); + allContributors.forEach(function (contributor) { + if (contributor.avatar_url && contributor.avatar_url.indexOf("avatars.githubusercontent.com") !== -1) { + contributor.avatar_url += "size=30"; + } + }); + + $contributors.html(Mustache.render(ContributorsTemplate, allContributors)); // This is used to create an opacity transition when each image is loaded $contributors.find("img").one("load", function () { @@ -105,10 +144,10 @@ define(function (require, exports, module) { $(this).trigger("load"); } }); - }).fail(function () { + } else { $spinner.removeClass("spin"); $contributors.html(Mustache.render("

{{ABOUT_TEXT_LINE6}}

", Strings)); - }); + } } // Read "build number" SHAs off disk immediately at APP_READY, instead diff --git a/src/styles/brackets_patterns_override.less b/src/styles/brackets_patterns_override.less index 760f940c02c..502fe42b736 100644 --- a/src/styles/brackets_patterns_override.less +++ b/src/styles/brackets_patterns_override.less @@ -988,13 +988,14 @@ a[href^="http"] { a { text-decoration: none; } - } - .about-contributors img { - opacity: 0; - -webkit-transition: opacity 1s; - -moz-transition: opacity 1s; - -o-transition: opacity 1s; - transition: opacity 1s; + img { + opacity: 0; + -webkit-transition: opacity 1s; + transition: opacity 1s; + // fixed height and width: avoid increasing scrollbar while loading + height: 30px; + width: 30px; + } } } From 04ea7395aaa82b7a6afa17d4f403c4ecf17b8563 Mon Sep 17 00:00:00 2001 From: Marcel Gerber Date: Sun, 27 Apr 2014 21:53:54 +0200 Subject: [PATCH 2/6] Don't rely on GitHubs ? --- src/help/HelpCommandHandlers.js | 5 ++--- src/htmlContent/contributors-list.html | 2 +- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/src/help/HelpCommandHandlers.js b/src/help/HelpCommandHandlers.js index 9b46ab74d77..1687bc73a6a 100644 --- a/src/help/HelpCommandHandlers.js +++ b/src/help/HelpCommandHandlers.js @@ -123,9 +123,8 @@ define(function (require, exports, module) { contributorsCount = 0; allContributors.forEach(function (contributor) { - if (contributor.avatar_url && contributor.avatar_url.indexOf("avatars.githubusercontent.com") !== -1) { - contributor.avatar_url += "size=30"; - } + // remove any UrlParams delivered via the GitHub API + contributor.avatar_url = contributor.avatar_url.split("?")[0]; }); $contributors.html(Mustache.render(ContributorsTemplate, allContributors)); diff --git a/src/htmlContent/contributors-list.html b/src/htmlContent/contributors-list.html index 50559e72cfc..ee2b0efa948 100644 --- a/src/htmlContent/contributors-list.html +++ b/src/htmlContent/contributors-list.html @@ -1 +1 @@ -{{#.}}{{login}}{{/.}} +{{#.}}{{login}}{{/.}} From 51d78f0dda7fe25949807cc817c384328cff5b15 Mon Sep 17 00:00:00 2001 From: Marcel Gerber Date: Sun, 4 May 2014 01:18:29 +0200 Subject: [PATCH 3/6] Code review changes --- src/brackets.config.json | 2 +- src/config.json | 2 +- src/help/HelpCommandHandlers.js | 23 +++++++++++----------- src/styles/brackets_patterns_override.less | 4 ---- 4 files changed, 14 insertions(+), 17 deletions(-) diff --git a/src/brackets.config.json b/src/brackets.config.json index cf0d700a753..72ea359d8d5 100644 --- a/src/brackets.config.json +++ b/src/brackets.config.json @@ -14,7 +14,7 @@ "twitter_url" : "https://twitter.com/brackets", "troubleshoot_url" : "https://github.com/adobe/brackets/wiki/Troubleshooting#wiki-livedev", "twitter_name" : "@brackets", - "contributors_url" : "https://api.github.com/repos/adobe/brackets/contributors", + "contributors_url" : "https://api.github.com/repos/adobe/brackets/contributors?per_page={0}&page={1}", "extension_listing_url" : "", "extension_registry" : "https://s3.amazonaws.com/extend.brackets/registry.json", "extension_url" : "https://s3.amazonaws.com/extend.brackets/{0}/{0}-{1}.zip", diff --git a/src/config.json b/src/config.json index f88388902fc..3d625a6adeb 100644 --- a/src/config.json +++ b/src/config.json @@ -13,7 +13,7 @@ "twitter_url": "https://twitter.com/brackets", "troubleshoot_url": "https://github.com/adobe/brackets/wiki/Troubleshooting#wiki-livedev", "twitter_name": "@brackets", - "contributors_url": "https://api.github.com/repos/adobe/brackets/contributors", + "contributors_url": "https://api.github.com/repos/adobe/brackets/contributors?per_page={0}&page={1}", "extension_listing_url": "", "extension_registry": "https://s3.amazonaws.com/extend.brackets/registry.json", "extension_url": "https://s3.amazonaws.com/extend.brackets/{0}/{0}-{1}.zip", diff --git a/src/help/HelpCommandHandlers.js b/src/help/HelpCommandHandlers.js index 1687bc73a6a..d0213e3dccf 100644 --- a/src/help/HelpCommandHandlers.js +++ b/src/help/HelpCommandHandlers.js @@ -29,16 +29,16 @@ define(function (require, exports, module) { "use strict"; var AppInit = require("utils/AppInit"), - Global = require("utils/Global"), BuildInfoUtils = require("utils/BuildInfoUtils"), - Commands = require("command/Commands"), CommandManager = require("command/CommandManager"), + Commands = require("command/Commands"), Dialogs = require("widgets/Dialogs"), - Strings = require("strings"), - UpdateNotification = require("utils/UpdateNotification"), FileUtils = require("file/FileUtils"), + Global = require("utils/Global"), NativeApp = require("utils/NativeApp"), + Strings = require("strings"), StringUtils = require("utils/StringUtils"), + UpdateNotification = require("utils/UpdateNotification"), AboutDialogTemplate = require("text!htmlContent/about-dialog.html"), ContributorsTemplate = require("text!htmlContent/contributors-list.html"); @@ -76,15 +76,16 @@ define(function (require, exports, module) { Dialogs.showModalDialogUsingTemplate(Mustache.render(AboutDialogTemplate, templateVars)); // Get containers - var $dlg = $(".about-dialog.instance"), - $contributors = $dlg.find(".about-contributors"), - $spinner = $dlg.find(".spinner"), + var $dlg = $(".about-dialog.instance"), + $contributors = $dlg.find(".about-contributors"), + $spinner = $dlg.find(".spinner"), + allContributors = [], contributorsUrl = brackets.config.contributors_url, + perPage = 100, page, - allContributors = [], data; - if (contributorsUrl.indexOf("api.github.com") !== -1) { + if (contributorsUrl.indexOf("{1}") !== -1) { // pagination enabled page = 1; } @@ -94,7 +95,7 @@ define(function (require, exports, module) { var contributors; if (page) { - url += "?per_page=100&page=" + page; + url = StringUtils.format(url, perPage, page); } $.ajax({ url: url, @@ -115,7 +116,7 @@ define(function (require, exports, module) { if (page) { page++; } - } while (page && data.length === 100); + } while (page && data.length === perPage); if (allContributors.length) { // Populate the contributors data diff --git a/src/styles/brackets_patterns_override.less b/src/styles/brackets_patterns_override.less index 502fe42b736..2be5c0d1b22 100644 --- a/src/styles/brackets_patterns_override.less +++ b/src/styles/brackets_patterns_override.less @@ -990,11 +990,7 @@ a[href^="http"] { } img { opacity: 0; - -webkit-transition: opacity 1s; transition: opacity 1s; - // fixed height and width: avoid increasing scrollbar while loading - height: 30px; - width: 30px; } } } From 36114f38056377488c537a47bf1f500741b231a6 Mon Sep 17 00:00:00 2001 From: Marcel Gerber Date: Sun, 4 May 2014 01:51:40 +0200 Subject: [PATCH 4/6] Code review changes --- src/help/HelpCommandHandlers.js | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/help/HelpCommandHandlers.js b/src/help/HelpCommandHandlers.js index d0213e3dccf..a6efdb0e98d 100644 --- a/src/help/HelpCommandHandlers.js +++ b/src/help/HelpCommandHandlers.js @@ -42,6 +42,9 @@ define(function (require, exports, module) { AboutDialogTemplate = require("text!htmlContent/about-dialog.html"), ContributorsTemplate = require("text!htmlContent/contributors-list.html"); + /** @const This is the thirdparty API's (GitHub) maximum contributors per page limit */ + var CONTRIBUTORS_PER_PAGE = 100; + var buildInfo; @@ -81,9 +84,8 @@ define(function (require, exports, module) { $spinner = $dlg.find(".spinner"), allContributors = [], contributorsUrl = brackets.config.contributors_url, - perPage = 100, - page, - data; + data, + page; if (contributorsUrl.indexOf("{1}") !== -1) { // pagination enabled page = 1; @@ -95,7 +97,7 @@ define(function (require, exports, module) { var contributors; if (page) { - url = StringUtils.format(url, perPage, page); + url = StringUtils.format(url, CONTRIBUTORS_PER_PAGE, page); } $.ajax({ url: url, @@ -116,7 +118,7 @@ define(function (require, exports, module) { if (page) { page++; } - } while (page && data.length === perPage); + } while (page && data.length === CONTRIBUTORS_PER_PAGE); if (allContributors.length) { // Populate the contributors data From 07194ca44bee43ab3c9c190d0cd24e84fca9e6d2 Mon Sep 17 00:00:00 2001 From: Marcel Gerber Date: Sun, 4 May 2014 02:53:34 +0200 Subject: [PATCH 5/6] Use deferreds --- src/help/HelpCommandHandlers.js | 97 +++++++++++++++++---------------- 1 file changed, 50 insertions(+), 47 deletions(-) diff --git a/src/help/HelpCommandHandlers.js b/src/help/HelpCommandHandlers.js index a6efdb0e98d..695211dacbf 100644 --- a/src/help/HelpCommandHandlers.js +++ b/src/help/HelpCommandHandlers.js @@ -93,63 +93,66 @@ define(function (require, exports, module) { $spinner.addClass("spin"); - function loadContributorsPage(url, page) { - var contributors; - + function loadContributors(rawUrl, page, contributors, deferred) { + deferred = deferred || $.Deferred(); + contributors = contributors || []; + var url = rawUrl; if (page) { - url = StringUtils.format(url, CONTRIBUTORS_PER_PAGE, page); + url = StringUtils.format(rawUrl, CONTRIBUTORS_PER_PAGE, page); } + $.ajax({ url: url, - async: false, dataType: "json", - cache: false, - complete: function (data) { - contributors = (data && data.responseJSON) || []; - } - }); - return contributors; + cache: false + }) + .done(function (response) { + var data = response || []; + contributors = contributors.concat(data); + if (page && data.length === CONTRIBUTORS_PER_PAGE) { + loadContributors(rawUrl, page + 1, contributors, deferred); + } else { + deferred.resolve(contributors); + } + }) + .fail(function () { + deferred.reject(contributors); + }); + return deferred; } - // Get all the project contributors - do { - data = loadContributorsPage(contributorsUrl, page); - allContributors = allContributors.concat(data); - if (page) { - page++; - } - } while (page && data.length === CONTRIBUTORS_PER_PAGE); + loadContributors(contributorsUrl, page) // Load the contributors + .done(function (allContributors) { + // Populate the contributors data + var totalContributors = allContributors.length, + contributorsCount = 0; - if (allContributors.length) { - // Populate the contributors data - var totalContributors = allContributors.length, - contributorsCount = 0; - - allContributors.forEach(function (contributor) { - // remove any UrlParams delivered via the GitHub API - contributor.avatar_url = contributor.avatar_url.split("?")[0]; - }); + allContributors.forEach(function (contributor) { + // remove any UrlParams delivered via the GitHub API + contributor.avatar_url = contributor.avatar_url.split("?")[0]; + }); + + $contributors.html(Mustache.render(ContributorsTemplate, allContributors)); - $contributors.html(Mustache.render(ContributorsTemplate, allContributors)); - - // This is used to create an opacity transition when each image is loaded - $contributors.find("img").one("load", function () { - $(this).css("opacity", 1); - - // Count the contributors loaded and hide the spinner once all are loaded - contributorsCount++; - if (contributorsCount >= totalContributors) { - $spinner.removeClass("spin"); - } - }).each(function () { - if (this.complete) { - $(this).trigger("load"); - } + // This is used to create an opacity transition when each image is loaded + $contributors.find("img").one("load", function () { + $(this).css("opacity", 1); + + // Count the contributors loaded and hide the spinner once all are loaded + contributorsCount++; + if (contributorsCount >= totalContributors) { + $spinner.removeClass("spin"); + } + }).each(function () { + if (this.complete) { + $(this).trigger("load"); + } + }); + }) + .fail(function () { + $spinner.removeClass("spin"); + $contributors.html(Mustache.render("

{{ABOUT_TEXT_LINE6}}

", Strings)); }); - } else { - $spinner.removeClass("spin"); - $contributors.html(Mustache.render("

{{ABOUT_TEXT_LINE6}}

", Strings)); - } } // Read "build number" SHAs off disk immediately at APP_READY, instead From 6eee20ee22aae0ce2439d0996e25b656a28b5f2a Mon Sep 17 00:00:00 2001 From: Marcel Gerber Date: Sun, 4 May 2014 13:36:59 +0200 Subject: [PATCH 6/6] Code review changes again --- src/help/HelpCommandHandlers.js | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/src/help/HelpCommandHandlers.js b/src/help/HelpCommandHandlers.js index 695211dacbf..9899df91041 100644 --- a/src/help/HelpCommandHandlers.js +++ b/src/help/HelpCommandHandlers.js @@ -82,9 +82,7 @@ define(function (require, exports, module) { var $dlg = $(".about-dialog.instance"), $contributors = $dlg.find(".about-contributors"), $spinner = $dlg.find(".spinner"), - allContributors = [], contributorsUrl = brackets.config.contributors_url, - data, page; if (contributorsUrl.indexOf("{1}") !== -1) { // pagination enabled @@ -94,12 +92,9 @@ define(function (require, exports, module) { $spinner.addClass("spin"); function loadContributors(rawUrl, page, contributors, deferred) { - deferred = deferred || $.Deferred(); + deferred = deferred || new $.Deferred(); contributors = contributors || []; - var url = rawUrl; - if (page) { - url = StringUtils.format(rawUrl, CONTRIBUTORS_PER_PAGE, page); - } + var url = StringUtils.format(rawUrl, CONTRIBUTORS_PER_PAGE, page); $.ajax({ url: url, @@ -107,18 +102,21 @@ define(function (require, exports, module) { cache: false }) .done(function (response) { - var data = response || []; - contributors = contributors.concat(data); - if (page && data.length === CONTRIBUTORS_PER_PAGE) { + contributors = contributors.concat(response || []); + if (page && response.length === CONTRIBUTORS_PER_PAGE) { loadContributors(rawUrl, page + 1, contributors, deferred); } else { deferred.resolve(contributors); } }) .fail(function () { - deferred.reject(contributors); + if (contributors.length) { // we weren't able to fetch this page, but previous fetches were successful + deferred.resolve(contributors); + } else { + deferred.reject(); + } }); - return deferred; + return deferred.promise(); } loadContributors(contributorsUrl, page) // Load the contributors