From b2d78bcc166cd340f913fae6efe48599ed994180 Mon Sep 17 00:00:00 2001 From: gorhill Date: Tue, 9 Sep 2014 10:53:47 -0400 Subject: [PATCH] code review --- 3p-filters.html | 10 +-- css/3p-filters.css | 45 ++++++++------ js/3p-filters.js | 147 +++++++++++++++++++++++---------------------- 3 files changed, 107 insertions(+), 95 deletions(-) diff --git a/3p-filters.html b/3p-filters.html index d8cbb3c47..a8c3d0253 100644 --- a/3p-filters.html +++ b/3p-filters.html @@ -10,15 +10,15 @@ - + @@ -29,7 +29,7 @@

-
+
diff --git a/css/3p-filters.css b/css/3p-filters.css index 8c2379a36..9e3b009af 100644 --- a/css/3p-filters.css +++ b/css/3p-filters.css @@ -2,6 +2,12 @@ ul { padding: 0; list-style-type: none; } +ul#options { + margin-top: 0; + } +ul#options li { + margin-bottom: 0.5em; + } ul#lists { margin: 0.5em 0 0 0; padding-__MSG_@@bidi_end_edge__: 0em; @@ -33,40 +39,41 @@ li.listDetails > a:nth-of-type(2) { opacity: 0.5; } /* I designed the button with: http://charliepark.org/bootstrap_buttons/ */ -button { - border: 1px solid transparent; - border-radius: 3px; - border-color: #dddddd #dddddd hsl(36, 0%, 85%); +button.custom { padding: 5px; - background-color: hsl(36, 0%, 72%); - background-repeat: repeat-x; - background-image: linear-gradient(#f2f2f2, #dddddd); - color: #aaa; - } -button.enabled { + border: 1px solid transparent; border-color: #80b3ff #80b3ff hsl(216, 100%, 75%); + border-radius: 3px; background-color: hsl(216, 100%, 75%); background-image: linear-gradient(#a8cbff, #80b3ff); + background-repeat: repeat-x; color: #222; cursor: pointer; opacity: 0.8; } -button.reloadAll.enabled { +button.custom.disabled { + border-color: #dddddd #dddddd hsl(36, 0%, 85%); + background-color: hsl(36, 0%, 72%); + background-image: linear-gradient(#f2f2f2, #dddddd); + color: #aaa; + pointer-events: none; + } +button.custom:hover { + opacity: 1.0; + } +button.custom.reloadAll:not(.disabled) { border-color: #ffcc7f #ffcc7f hsl(36, 100%, 73%); background-color: hsl(36, 100%, 75%); background-image: linear-gradient(#ffdca8, #ffcc7f); } -button.enabled:hover { - opacity: 1.0; - } #buttonApply { - display: none; position: fixed; + display: initial; top: 1em; __MSG_@@bidi_end_edge__: 1em; } -#buttonApply.enabled { - display: initial; +#buttonApply.disabled { + display: none; } span.status { margin: 0; @@ -100,7 +107,7 @@ span.obsolete { height: 8em; white-space: nowrap; } -body #loadingOverlay { +body #busyOverlay { position: fixed; top: 0; right: 0; @@ -112,6 +119,6 @@ body #loadingOverlay { display: none; z-index: 1000; } -body.loading #loadingOverlay { +body.busy #busyOverlay { display: block; } diff --git a/js/3p-filters.js b/js/3p-filters.js index 899f07122..4f56f8c57 100644 --- a/js/3p-filters.js +++ b/js/3p-filters.js @@ -35,8 +35,8 @@ var cacheWasPurged = false; var needUpdate = false; var hasCachedContent = false; -var reExternalAsset = /^https?:\/\/[a-z0-9]+/; -var reRepo3rdPartyAsset = /^assets\/thirdparties\/([^\/]+)/; +var re3rdPartyExternalAsset = /^https?:\/\/[a-z0-9]+/; +var re3rdPartyRepoAsset = /^assets\/thirdparties\/([^\/]+)/; /******************************************************************************/ @@ -45,7 +45,6 @@ messaging.start('3p-filters.js'); var onMessage = function(msg) { switch ( msg.what ) { case 'loadUbiquitousBlacklistCompleted': - uDom('body').toggleClass('loading', false); renderBlacklists(); break; @@ -73,6 +72,8 @@ var renderNumber = function(value) { // TODO: get rid of background page dependencies var renderBlacklists = function() { + uDom('body').toggleClass('busy', true); + var µb = getµb(); // Assemble a pretty blacklist name if possible @@ -91,7 +92,7 @@ var renderBlacklists = function() { if ( blacklistHref.indexOf('assets/thirdparties/') !== 0 ) { return ''; } - var matches = reRepo3rdPartyAsset.exec(blacklistHref); + var matches = re3rdPartyRepoAsset.exec(blacklistHref); if ( matches === null || matches.length !== 2 ) { return ''; } @@ -110,12 +111,68 @@ var renderBlacklists = function() { return html.join(''); }; - var listStatsTemplate = chrome.i18n.getMessage('3pListsOfBlockedHostsPerListStats'); var purgeButtontext = chrome.i18n.getMessage('3pExternalListPurge'); var updateButtontext = chrome.i18n.getMessage('3pExternalListNew'); var obsoleteButtontext = chrome.i18n.getMessage('3pExternalListObsolete'); + var liTemplate = [ + '
  • ', + '', + ' ', + '', + '{{name}}', + '\u200E', + '{{homeURL}}', + ': ', + '', + chrome.i18n.getMessage('3pListsOfBlockedHostsPerListStats'), + '' + ].join(''); - var htmlFromBranch = function(groupKey, listKeys, lists) { + var htmlFromLeaf = function(listKey) { + var html = []; + var list = listDetails.available[listKey]; + var li = liTemplate + .replace('{{checked}}', list.off ? '' : 'checked') + .replace('{{URL}}', encodeURI(listKey)) + .replace('{{name}}', htmlFromListName(list.title, listKey)) + .replace('{{homeURL}}', htmlFromHomeURL(listKey)) + .replace('{{used}}', !list.off && !isNaN(+list.entryUsedCount) ? renderNumber(list.entryUsedCount) : '0') + .replace('{{total}}', !isNaN(+list.entryCount) ? renderNumber(list.entryCount) : '?'); + html.push(li); + // https://github.com/gorhill/uBlock/issues/104 + var asset = listDetails.cache[listKey]; + if ( asset === undefined ) { + return html.join('\n'); + } + // Update status + if ( list.off !== true ) { + var obsolete = asset.repoObsolete || + asset.cacheObsolete || + asset.cached !== true && re3rdPartyExternalAsset.test(listKey); + if ( obsolete ) { + html.push( + ' ', + '', + asset.repoObsolete ? updateButtontext : obsoleteButtontext, + '' + ); + needUpdate = true; + } + } + // In cache + if ( asset.cached ) { + html.push( + ' ', + '', + purgeButtontext, + '' + ); + hasCachedContent = true; + } + return html.join('\n'); + }; + + var htmlFromBranch = function(groupKey, listKeys) { var html = [ '
  • ', chrome.i18n.getMessage('3pGroup' + groupKey.charAt(0).toUpperCase() + groupKey.slice(1)), @@ -125,63 +182,10 @@ var renderBlacklists = function() { return html.join(''); } listKeys.sort(function(a, b) { - return lists[a].title.localeCompare(lists[b].title); + return listDetails.available[a].title.localeCompare(listDetails.available[b].title); }); - var listEntryTemplate = [ - '
  • ', - '', - ' ', - '', - '{{name}}', - '\u200E', - '{{homeURL}}', - ': ', - '', - listStatsTemplate, - '' - ].join(''); - var listKey, list, listEntry, entryDetails, obsolete; for ( var i = 0; i < listKeys.length; i++ ) { - listKey = listKeys[i]; - list = lists[listKey]; - listEntry = listEntryTemplate - .replace('{{checked}}', list.off ? '' : 'checked') - .replace('{{URL}}', encodeURI(listKey)) - .replace('{{name}}', htmlFromListName(list.title, listKey)) - .replace('{{homeURL}}', htmlFromHomeURL(listKey)) - .replace('{{used}}', !list.off && !isNaN(+list.entryUsedCount) ? renderNumber(list.entryUsedCount) : '0') - .replace('{{total}}', !isNaN(+list.entryCount) ? renderNumber(list.entryCount) : '?'); - html.push(listEntry); - // https://github.com/gorhill/uBlock/issues/104 - entryDetails = listDetails.cache[listKey]; - if ( entryDetails === undefined ) { - continue; - } - // Update status - if ( list.off !== true ) { - obsolete = entryDetails.repoObsolete || - entryDetails.cacheObsolete || - entryDetails.cached !== true && reExternalAsset.test(listKey); - if ( obsolete ) { - html.push( - ' ', - '', - entryDetails.repoObsolete ? updateButtontext : obsoleteButtontext, - '' - ); - needUpdate = true; - } - } - // In cache - if ( entryDetails.cached ) { - html.push( - ' ', - '', - purgeButtontext, - '' - ); - hasCachedContent = true; - } + html.push(htmlFromLeaf(listKeys[i])); } html.push(''); return html.join(''); @@ -207,14 +211,15 @@ var renderBlacklists = function() { }; var onListsReceived = function(details) { + // Before all, set context vars listDetails = details; cosmeticSwitch = details.cosmetic; needUpdate = false; hasCachedContent = false; - var lists = details.available; + // Visually split the filter lists in purpose-based groups var html = []; - var groups = groupsFromLists(lists); + var groups = groupsFromLists(details.available); var groupKey, i; var groupKeys = [ 'default', @@ -228,14 +233,14 @@ var renderBlacklists = function() { ]; for ( i = 0; i < groupKeys.length; i++ ) { groupKey = groupKeys[i]; - html.push(htmlFromBranch(groupKey, groups[groupKey], lists)); + html.push(htmlFromBranch(groupKey, groups[groupKey])); delete groups[groupKey]; } // For all groups not covered above (if any left) groupKeys = Object.keys(groups); for ( i = 0; i < groupKeys.length; i++ ) { groupKey = groupKeys[i]; - html.push(htmlFromBranch(groupKey, groups[groupKey], lists)); + html.push(htmlFromBranch(groupKey, groups[groupKey])); delete groups[groupKey]; } @@ -307,9 +312,10 @@ var listsContentChanged = function() { // This is to give a visual hint that the selection of blacklists has changed. var updateWidgets = function() { - uDom('#buttonApply').toggleClass('enabled', listsSelectionChanged()); - uDom('#buttonUpdate').toggleClass('enabled', listsContentChanged()); - uDom('#buttonPurgeAll').toggleClass('enabled', hasCachedContent); + uDom('#buttonApply').toggleClass('disabled', !listsSelectionChanged()); + uDom('#buttonUpdate').toggleClass('disabled', !listsContentChanged()); + uDom('#buttonPurgeAll').toggleClass('disabled', !hasCachedContent); + uDom('body').toggleClass('busy', false); }; /******************************************************************************/ @@ -356,9 +362,9 @@ var onPurgeClicked = function() { /******************************************************************************/ var reloadAll = function(update) { - // Loading may take a while when resoruces are fetched from remote + // Loading may take a while when resources are fetched from remote // servers. We do not want the user to force reload while we are reloading. - uDom('body').toggleClass('loading', true); + uDom('body').toggleClass('busy', true); // Reload blacklists messaging.tell({ @@ -465,7 +471,6 @@ var externalListsApplyHandler = function() { /******************************************************************************/ uDom.onLoad(function() { - // Handle user interaction uDom('#autoUpdate').on('change', autoUpdateCheckboxChanged); uDom('#parseCosmeticFilters').on('change', cosmeticSwitchChanged); uDom('#buttonApply').on('click', buttonApplyHandler);