Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code

Unified Diff: new-options.js

Issue 29445590: Issue 5255 - Advanced tab (HTML, strings and functionality) (Closed)
Patch Set: Addressed notes from weekly(ish) meeting and small collection fix Created June 14, 2017, 11:58 a.m.
Use n/p to move between diff chunks; N/P to move between comments.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: new-options.js
===================================================================
--- a/new-options.js
+++ b/new-options.js
@@ -38,6 +38,8 @@
"options_filterList_lastDownload_checksumMismatch"]
]);
+ const whitelistedDomainRegexp = /^@@\|\|([^/:]+)\^\$document$/;
+
function Collection(details)
{
this.details = details;
@@ -100,7 +102,8 @@
for (let j = 0; j < this.details.length; j++)
{
- let table = E(this.details[j].id);
+ let detail = this.details[j];
+ let table = E(detail.id);
let template = table.querySelector("template");
let listItem = document.createElement("li");
listItem.appendChild(document.importNode(template.content, true));
@@ -126,13 +129,15 @@
}
this._setEmpty(table, null);
- if (table.hasChildNodes())
+ if (table.children.length > 0)
{
- table.insertBefore(listItem,
- table.childNodes[this.items.indexOf(item)]);
+ let beforeIndex = this.items.indexOf(item) + (detail.useHeader == true);
+ table.insertBefore(listItem, table.children[beforeIndex]);
}
Thomas Greiner 2017/07/07 13:01:09 What's the reason for those changes? I didn't noti
saroyanm 2017/07/10 11:38:00 Header is column name. I used naming header from "
saroyanm 2017/07/10 13:27:43 I'll move the column names row out of the list for
saroyanm 2017/07/12 15:29:02 Done.
else
+ {
table.appendChild(listItem);
+ }
this.updateItem(item);
}
return length;
@@ -200,9 +205,8 @@
control.setAttribute("disabled", true);
}
- let dateElement = element.querySelector(".date");
- let timeElement = element.querySelector(".time");
- if (dateElement && timeElement)
+ let lastUpdateElem = element.querySelector(".last-update");
+ if (lastUpdateElem)
{
let message = element.querySelector(".message");
if (item.isDownloading)
@@ -222,9 +226,40 @@
}
else if (item.lastDownload > 0)
{
- let dateTime = i18nFormatDateTime(item.lastDownload * 1000);
- dateElement.textContent = dateTime[0];
- timeElement.textContent = dateTime[1];
+ let timer = setInterval(lastUpdateLive, 60000);
Thomas Greiner 2017/07/07 13:01:09 As far as I see, the spec doesn't say that this ti
saroyanm 2017/07/10 11:38:02 I do remember that this was discussed during our m
saroyanm 2017/07/12 15:29:01 Removed live update
+ let lastUpdate = item.lastDownload * 1000;
+ function lastUpdateLive()
+ {
+ let sinceUpdate = Date.now() - lastUpdate;
+ if(sinceUpdate > 86400000)
Thomas Greiner 2017/07/07 13:01:10 Detail: Usually, we create constants to make it mo
saroyanm 2017/07/10 11:38:01 Agree
saroyanm 2017/07/12 15:29:02 Done.
+ {
+ let lastUpdateDate = new Date(item.lastDownload * 1000);
+ let monthName = lastUpdateDate.toLocaleString(
+ document.documentElement.lang, { month: "short" });
Thomas Greiner 2017/07/07 13:01:10 Actually, while reviewing #5278 I noticed that we
Thomas Greiner 2017/07/07 13:01:10 Coding style: This violates the rule "object-curly
saroyanm 2017/07/10 11:38:00 Good point, I'll run eslint before uploading next
saroyanm 2017/07/10 11:38:01 Acknowledged, checked specifications as well I thi
saroyanm 2017/07/12 15:29:01 Done.
saroyanm 2017/07/12 15:29:01 Done.
+ let day = lastUpdateDate.getDate();
+ day = day < 10 ? "0" + day : day;
+ lastUpdateElem.textContent = day + " " + monthName + " " +
+ lastUpdateDate.getFullYear();
+ clearInterval(timer);
+ return;
+ }
+ else if (sinceUpdate > 3600000)
+ {
+ lastUpdateElem.textContent = Math.round(sinceUpdate / 3600000) +
+ " " + getMessage("options_filterList_hours");
+ }
+ else if (sinceUpdate > 60000)
+ {
+ lastUpdateElem.textContent = Math.round(sinceUpdate / 60000) +
+ " " + getMessage("options_filterList_minutes");
+ }
+ else
+ {
+ lastUpdateElem.textContent =
+ getMessage("options_filterList_just");
+ }
+ }
+ lastUpdateLive();
element.classList.remove("show-message");
}
}
@@ -315,16 +350,11 @@
emptyText: "options_whitelisted_empty"
}
]);
- collections.customFilters = new Collection([
- {
- id: "custom-filters-table",
- emptyText: "options_customFilters_empty"
- }
- ]);
collections.filterLists = new Collection([
{
id: "all-filter-lists-table",
- useOriginalTitle: true
+ useOriginalTitle: true,
+ useHeader: true
}
]);
@@ -378,18 +408,46 @@
function updateFilter(filter)
{
- let match = filter.text.match(/^@@\|\|([^/:]+)\^\$document$/);
+ let match = filter.text.match(whitelistedDomainRegexp);
if (match && !filtersMap[filter.text])
{
filter.title = match[1];
collections.whitelist.addItem(filter);
}
else
- collections.customFilters.addItem(filter);
+ addCustomFilter(filter);
filtersMap[filter.text] = filter;
}
+ function addCustomFilter(filter)
+ {
+ customFiltersView("read");
Thomas Greiner 2017/07/07 13:01:10 Detail: The name is ambiguous because it doesn't s
saroyanm 2017/07/10 11:38:01 Agree.
saroyanm 2017/07/12 15:29:01 Done.
+
+ E("custom-filters-box").appendChild(new Option(filter.text, filter.text));
+ }
+
+ function removeCustomFilter(text)
+ {
+ let list = E("custom-filters-box");
+ let selector = "option[value=" + CSS.escape(text) + "]";
+ for (let option of list.querySelectorAll(selector))
+ list.removeChild(option);
+
+ if (E("custom-filters-box").options.length == 0)
+ customFiltersView("empty")
+ }
+
+ function convertToRawFormat(event)
+ {
+ let filters = [];
+
+ for (let option of E("custom-filters-box").options)
+ filters.push(option.value);
+
+ document.getElementById("custom-filters-raw").value = filters.join("\n");
+ }
+
function loadRecommendations()
{
fetch("subscriptions.xml")
@@ -474,8 +532,11 @@
location.hash = id;
}
- function execAction(action, element)
+ function execAction(action, element, event)
{
+ if (element.getAttribute("href") == "#")
Thomas Greiner 2017/07/07 13:01:10 Detail: Why do you even set the "href" attribute i
saroyanm 2017/07/10 11:38:00 Good question, I think we don't need href attribut
saroyanm 2017/07/12 15:29:02 Done.
+ event.preventDefault();
+
switch (action)
{
case "add-domain-exception":
@@ -493,7 +554,7 @@
break;
}
case "cancel-custom-filters":
- E("custom-filters").classList.remove("mode-edit");
+ customFiltersView("read");
break;
case "cancel-domain-exception":
E("whitelisting-textbox").value = "";
@@ -504,7 +565,7 @@
closeDialog();
break;
case "edit-custom-filters":
- editCustomFilters();
+ customFiltersView("write");
break;
case "edit-domain-exception":
document.querySelector("#whitelisting .controls").classList
@@ -553,7 +614,7 @@
},
() =>
{
- E("custom-filters").classList.remove("mode-edit");
+ customFiltersView("read");
});
break;
case "switch-tab":
@@ -599,6 +660,36 @@
}
}
+ function customFiltersView(mode)
Thomas Greiner 2017/07/07 13:01:09 Detail: I noticed that you've call `E("custom-filt
Thomas Greiner 2017/07/07 13:01:09 You're trying to switch between three modes but ea
saroyanm 2017/07/10 11:38:00 ".mode-edit" Will be consistent with other impleme
saroyanm 2017/07/10 11:38:00 Agree.
saroyanm 2017/07/12 15:29:02 Done.
saroyanm 2017/07/12 15:29:02 Done.
+ {
+ switch (mode)
+ {
+ case "read":
+ if (E("custom-filters-box").options.length == 0)
+ {
+ customFiltersView("empty");
+ }
+ else
+ {
+ E("custom-filters").classList.add("mode-view");
Thomas Greiner 2017/07/07 13:01:10 Detail: Why the different naming? I do like that y
saroyanm 2017/07/10 11:38:00 Acknowledged, will make them consistent
saroyanm 2017/07/12 15:29:02 Done.
+ E("custom-filters").classList.remove("mode-edit");
+ E("custom-filters").classList.remove("mode-empty");
Thomas Greiner 2017/07/07 13:01:09 Detail: Note that you can pass multiple arguments
saroyanm 2017/07/10 11:38:01 Acknowledged.
+ }
+ break;
+ case "write":
+ convertToRawFormat();
+ E("custom-filters").classList.remove("mode-view");
+ E("custom-filters").classList.add("mode-edit");
+ E("custom-filters").classList.remove("mode-empty");
+ break;
+ case "empty":
+ E("custom-filters").classList.remove("mode-view");
+ E("custom-filters").classList.remove("mode-edit");
+ E("custom-filters").classList.add("mode-empty");
+ break;
+ }
+ }
+
function onClick(e)
{
let context = document.querySelector(".show-context-menu");
@@ -612,7 +703,7 @@
actions = actions.split(",");
for (let action of actions)
{
- execAction(action, e.target);
+ execAction(action, e.target, e);
}
}
@@ -659,7 +750,7 @@
let actions = container.getAttribute("data-action").split(",");
for (let action of actions)
{
- execAction(action, element);
+ execAction(action, element, e);
}
}
@@ -684,10 +775,6 @@
let tabContentId = tab.getAttribute("aria-controls");
let tabContent = document.getElementById(tabContentId);
- // Select sub tabs
- if (tab.hasAttribute("data-subtab"))
- selectTabItem(tab.getAttribute("data-subtab"), tabContent, false);
-
if (tab && focus)
tab.focus();
@@ -757,12 +844,12 @@
}, false);
// Advanced tab
- let tweaks = document.querySelectorAll("#tweaks li[data-pref]");
- tweaks = Array.prototype.map.call(tweaks, (checkbox) =>
+ let customize = document.querySelectorAll("#customize li[data-pref]");
+ customize = Array.prototype.map.call(customize, (checkbox) =>
{
return checkbox.getAttribute("data-pref");
});
- for (let key of tweaks)
+ for (let key of customize)
{
getPref(key, (value) =>
{
@@ -778,26 +865,18 @@
hidePref("show_devtools_panel", !features.devToolsPanel);
});
- let filterTextbox = document.querySelector("#custom-filters-add input");
- placeholderValue = getMessage("options_customFilters_textbox_placeholder");
- filterTextbox.setAttribute("placeholder", placeholderValue);
- function addCustomFilters()
+ getDocLink("filterdoc", (link) =>
{
- let filterText = filterTextbox.value;
- sendMessageHandleErrors({
- type: "filters.add",
- text: filterText
- },
- () =>
- {
- filterTextbox.value = "";
- });
- }
- E("custom-filters-add").addEventListener("submit", (e) =>
+ E("link-filters").setAttribute("href", link);
+ });
+
+ getDocLink("subscriptions", (link) =>
{
- e.preventDefault();
- addCustomFilters();
- }, false);
+ setLinks("filter-lists-description", link);
+ });
+
+ E("custom-filters-raw").setAttribute("placeholder",
+ getMessage("options_customFilters_edit_placeholder", ["/ads/track/*"]));
// Help tab
getDocLink("faq", (link) =>
@@ -1024,8 +1103,11 @@
break;
case "removed":
let knownFilter = filtersMap[filter.text];
- collections.whitelist.removeItem(knownFilter);
- collections.customFilters.removeItem(knownFilter);
+ if (whitelistedDomainRegexp.test(knownFilter.text))
+ collections.whitelist.removeItem(knownFilter);
+ else
+ removeCustomFilter(filter.text);
+
delete filtersMap[filter.text];
updateShareLink();
break;

Powered by Google App Engine
This is Rietveld