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

Delta Between Two Patch Sets: static/js/main.js

Issue 29559620: Issue 5692 - Create Browser Selector with Browser Detection Component for help.eyeo.com (Closed) Base URL: https://hg.adblockplus.org/help.eyeo.com
Left Patch Set: Remove test class names from body Created Oct. 23, 2017, 4:14 p.m.
Right Patch Set: Re-add browser-select include, add scripts block Created Nov. 2, 2017, 2:29 p.m.
Left:
Right:
Use n/p to move between diff chunks; N/P to move between comments.
Jump to:
Left: Side by side diff | Download
Right: Side by side diff | Download
LEFTRIGHT
1 (function(){ 1 (function()
2 {
2 document.addEventListener("DOMContentLoaded", function() 3 document.addEventListener("DOMContentLoaded", function()
3 { 4 {
4 5
5 /************************************************************************** 6 /**************************************************************************
6 * General 7 * General
7 **************************************************************************/ 8 **************************************************************************/
8 9
9 // Change html class name from "no-js" to "js" 10 // Change html class name from "no-js" to "js"
10 document.documentElement.className = "js"; 11 document.documentElement.className = "js";
11 12
(...skipping 19 matching lines...) Expand all
31 } 32 }
32 33
33 /************************************************************************** 34 /**************************************************************************
34 * CustomSelect 35 * CustomSelect
35 **************************************************************************/ 36 **************************************************************************/
36 37
37 function CustomSelect(select) 38 function CustomSelect(select)
38 { 39 {
39 this.select = select; 40 this.select = select;
40 this.close(); 41 this.close();
41 select.addEventListener("click", this._onClick.bind(this), false); 42 this.select
43 .addEventListener("click", this._onClick.bind(this), false);
44 this.select
45 .addEventListener("focusout", this._onFocusOut.bind(this), false);
46 }
47
48 CustomSelect.prototype._onFocusOut = function()
49 {
50 // setTimeout to allow document.activeElement
51 // to move to newly focused element
52 setTimeout(function()
53 {
54 var newFocus = document.activeElement;
55
56 if (newFocus
57 .classList.contains("custom-select-selected") ||
58 newFocus
59 .classList.contains("custom-select-option") ||
60 newFocus
61 .parentElement
62 .classList.contains("custom-select-option"))
63 {
64 return;
65 }
66
67 this.close();
68
69 }.bind(this), 1);
42 } 70 }
43 71
44 CustomSelect.prototype._onClick = function(event) 72 CustomSelect.prototype._onClick = function(event)
45 { 73 {
46 if (event.target.classList.contains("custom-select-selected")) 74 if (event.target.classList.contains("custom-select-selected"))
47 { 75 {
48 var options = this.select.querySelector(".custom-select-options"); 76 var options = this.select.querySelector(".custom-select-options");
49 if (options.getAttribute("aria-hidden") == "true") 77 if (options.getAttribute("aria-hidden") == "true")
50 { 78 {
51 this.open(); 79 this.open();
(...skipping 146 matching lines...) Expand 10 before | Expand all | Expand 10 after
198 { 226 {
199 new Accordion(productTopicsAccordion); 227 new Accordion(productTopicsAccordion);
200 } 228 }
201 229
202 /************************************************************************** 230 /**************************************************************************
203 * BrowserSelect 231 * BrowserSelect
204 **************************************************************************/ 232 **************************************************************************/
205 233
206 function BrowserSelect(select) 234 function BrowserSelect(select)
207 { 235 {
208 this.browserSelect = select; 236 this.select = select;
juliandoucette 2017/10/24 12:12:28 NIT: You are repeating yourself by prefacing these
ire 2017/10/27 10:22:57 You're right. Done.
209 this.browserCustomSelect = new CustomSelect(this.browserSelect); 237 CustomSelect.apply(this, [this.select]);
juliandoucette 2017/10/24 12:12:28 NIT/Suggest: Looking at this again, I think that t
ire 2017/10/27 10:22:56 Done.
210 238
211 this.DEFAULT_BROWSER = "chrome";
212 this.BROWSER_STORAGE_KEY = "BROWSER"; 239 this.BROWSER_STORAGE_KEY = "BROWSER";
213 this.BROWSER_AUTODETECTED_STORAGE_KEY = "BROWSER_AUTODETECTED"; 240 this.BROWSER_AUTODETECTED_STORAGE_KEY = "BROWSER_AUTODETECTED";
214 241 this.SUPPORTED_BROWSERS = ["chrome", "opera", "samsungBrowser",
215 this.browserSelect 242 "yandexbrowser", "maxthon", "msie",
243 "msedge", "firefox", "ios", "safari"];
244 this.DEFAULT_BROWSER = "chrome";
245
246 this.select
216 .addEventListener("click", this._onClickOrKeyDown.bind(this), false); 247 .addEventListener("click", this._onClickOrKeyDown.bind(this), false);
217 248
218 this.browserSelect 249 this.select
219 .addEventListener("keydown", this._onClickOrKeyDown.bind(this), false); 250 .addEventListener("keydown", this._onClickOrKeyDown.bind(this), false);
220 251
221 var storedBrowser = localStorage.getItem(this.BROWSER_STORAGE_KEY); 252 var storedBrowser = localStorage.getItem(this.BROWSER_STORAGE_KEY);
juliandoucette 2017/10/24 12:12:28 NIT/Suggest: Place this above event listeners with
ire 2017/10/27 10:22:56 I think I prefer this being close to where it's us
222 if (storedBrowser) 253 if (storedBrowser) this.selectOption(storedBrowser);
juliandoucette 2017/10/24 12:12:29 NIT/Suggest: Our coding style permits braceless on
ire 2017/10/27 10:22:58 Done.
223 { 254 else this.detectBrowser();
224 this.selectOption(storedBrowser); 255 }
225 } 256
226 else 257 BrowserSelect.prototype = Object.create(CustomSelect.prototype);
227 { 258 BrowserSelect.prototype.constructor = BrowserSelect;
228 this.detectBrowser();
229 }
230 }
juliandoucette 2017/10/24 12:12:28 NIT: Missing ";" (the same applies elsewhere)
ire 2017/10/27 10:22:57 The ";" isn't used for the closing brace of functi
juliandoucette 2017/10/31 13:19:57 *facepalm* (because I commented on the only place
ire 2017/11/01 12:36:47 Haha :D
231 259
232 BrowserSelect.prototype.detectBrowser = function() 260 BrowserSelect.prototype.detectBrowser = function()
233 { 261 {
234 localStorage.setItem(this.BROWSER_AUTODETECTED_STORAGE_KEY, "true"); 262 for (var i = 0; i < this.SUPPORTED_BROWSERS.length; i++)
235 263 {
236 var browser; 264 var supportedBrowser = this.SUPPORTED_BROWSERS[i];
237 if (bowser.chrome) browser = "chrome"; 265 if (bowser[supportedBrowser])
juliandoucette 2017/10/24 12:12:29 NIT/Suggest: this.supportedBrowsers = ["chorme",
ire 2017/10/27 10:22:57 I need to check for the specific browser name to s
juliandoucette 2017/10/31 13:19:58 I did so by setting the variable `supportedBrowser
ire 2017/11/01 12:36:47 Ooo I see! I don't think I read your example prope
juliandoucette 2017/11/01 12:49:21 Why don't we use their names instead?
ire 2017/11/01 17:20:11 We could. Done.
238 else if (bowser.opera) browser = "opera"; 266 {
239 else if (bowser.coast) browser = "opera"; 267 localStorage.setItem(this.BROWSER_AUTODETECTED_STORAGE_KEY, "true");
240 else if (bowser.samsungBrowser) browser = "samsung"; 268 return this.selectOption(supportedBrowser);
241 else if (bowser.yandexbrowser) browser = "yandex"; 269 }
242 else if (bowser.maxthon) browser = "maxthon"; 270 }
243 else if (bowser.msie) browser = "ie"; 271
244 else if (bowser.msedge) browser = "edge"; 272 this.selectOption(this.DEFAULT_BROWSER);
245 else if (bowser.firefox) browser = "firefox"; 273 };
246 else if (bowser.ios) browser = "ios";
247 else if (bowser.safari) browser = "safari";
248 else
249 {
250 localStorage.removeItem(this.BROWSER_AUTODETECTED_STORAGE_KEY);
251 browser = this.DEFAULT_BROWSER;
252 }
253
254 this.selectOption(browser);
255 }
256 274
257 BrowserSelect.prototype.selectOption = function(browser) 275 BrowserSelect.prototype.selectOption = function(browser)
258 { 276 {
259 // Save value to Local Storage
juliandoucette 2017/10/24 12:12:29 NIT: I don't think that this comment is necessary?
ire 2017/10/27 10:22:56 Done.
260 localStorage.setItem(this.BROWSER_STORAGE_KEY, browser); 277 localStorage.setItem(this.BROWSER_STORAGE_KEY, browser);
261 278
262 // Change body class 279 // Change body class
263 var bodyClassList = Array.prototype.slice.call(document.body.classList); 280 var bodyClassList = Array.prototype.slice.call(document.body.classList);
juliandoucette 2017/10/24 12:12:27 document.body.classList has length. You don't need
ire 2017/10/27 10:22:57 I can't do it that way because when a class is rem
juliandoucette 2017/10/31 13:19:57 Acknowledged. Doh.
264 for (var i = 0; i < bodyClassList.length; i++) 281 for (var i = 0; i < bodyClassList.length; i++)
265 { 282 {
266 if (bodyClassList[i].indexOf('ua-') > -1) 283 if (bodyClassList[i].indexOf('ua-') > -1)
267 { 284 {
268 document.body.classList.remove(bodyClassList[i]); 285 document.body.classList.remove(bodyClassList[i]);
269 } 286 }
270 } 287 }
271 document.body.classList.add("ua-" + browser); 288 document.body.classList.add("ua-" + browser);
272 289
273 // Check selected option 290 // Check selected option
274 var selectedItem = this.browserSelect 291 var selectedItem = this.select
juliandoucette 2017/10/24 12:12:29 I think that we usually open the brace on the same
ire 2017/10/27 10:22:58 I moved the .querySelector line because it would b
juliandoucette 2017/10/31 13:19:58 I know. My comment still applies? We usually open
juliandoucette 2017/10/31 13:24:02 Can you advise us on this kzar?
kzar 2017/10/31 14:22:31 IIRC we don't have a ESLint rule for this case, so
ire 2017/11/01 12:36:48 Thanks kzar!
275 .querySelector("[data-value='" + browser + "']"); 292 .querySelector("[data-value='" + browser + "']");
276 selectedItem.setAttribute("aria-checked", "true"); 293 selectedItem.setAttribute("aria-checked", "true");
277 294
278 // Set selected option 295 // Set selected option
279 var selectedOption = document.createElement("li"); 296 var selectedOption = selectedItem.innerHTML;
juliandoucette 2017/10/24 12:12:28 You don't seem to be using this element (just mani
ire 2017/10/27 10:22:57 You're right, don't notice that :p Done.
280 selectedOption.innerHTML = selectedItem.innerHTML;
281 297
282 if (localStorage.getItem(this.BROWSER_AUTODETECTED_STORAGE_KEY)) 298 if (localStorage.getItem(this.BROWSER_AUTODETECTED_STORAGE_KEY))
283 { 299 {
284 var autodetected = document 300 var autodetected = document
285 .getElementById('browser-select-autodetected') 301 .getElementById('browser-select-autodetected')
286 .innerHTML; 302 .innerHTML;
287 selectedOption.innerHTML += "<span class='muted'>(" + autodetected + ")< /span>"; 303 selectedOption += "<span class='muted'>(" + autodetected + ")</span>";
288 } 304 }
289 305
290 this.browserSelect 306 this.select
291 .querySelector(".custom-select-selected") 307 .querySelector(".custom-select-selected")
292 .innerHTML = selectedOption.innerHTML; 308 .innerHTML = selectedOption;
293 } 309 };
294 310
295 BrowserSelect.prototype._onClickOrKeyDown = function(event) 311 BrowserSelect.prototype._onClickOrKeyDown = function(event)
296 { 312 {
297 if (!event.target.classList.contains("custom-select-option")) return; 313 if (!event.target.classList.contains("custom-select-option")) return;
298 314
299 var IS_ENTER_KEY = event.key == "Enter" || event.keyCode == 13; 315 var IS_ENTER_KEY = event.key == "Enter" || event.keyCode == 13;
300 if (event.keyCode && !IS_ENTER_KEY) return; 316 if (event.keyCode && !IS_ENTER_KEY) return;
301 317
302 localStorage.removeItem(this.BROWSER_AUTODETECTED_STORAGE_KEY); 318 localStorage.removeItem(this.BROWSER_AUTODETECTED_STORAGE_KEY);
303 319
304 // Uncheck previously checked option 320 // Uncheck previously checked option
305 this.browserSelect 321 this.select
306 .querySelector("[aria-checked='true']") 322 .querySelector("[aria-checked='true']")
307 .setAttribute("aria-checked", "false"); 323 .setAttribute("aria-checked", "false");
308 324
309 this.selectOption(event.target.getAttribute("data-value")); 325 this.selectOption(event.target.getAttribute("data-value"));
310 326
311 // Close Select 327 this.close();
312 this.browserCustomSelect.close(); 328 };
313 }
314 329
315 var browserSelect = document.getElementById("browser-select"); 330 var browserSelect = document.getElementById("browser-select");
316 if (browserSelect) 331 if (browserSelect)
317 { 332 {
318 new BrowserSelect(browserSelect); 333 new BrowserSelect(browserSelect);
319 } 334 }
320 335
321 }, false); 336 }, false);
322 }()); 337 }());
LEFTRIGHT

Powered by Google App Engine
This is Rietveld