includes/browser-select.tmpl - Issue 29559620: Issue 5692 - Create Browser Selector with Browser Detection Component for help.eyeo.com

Side by Side Diff: includes/browser-select.tmpl

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

Patch Set: Rebased Created Oct. 11, 2017, 5:20 p.m.

Left:
Right:

Use n/p to move between diff chunks; N/P to move between comments.

Jump to:

View unified diff | Download patch

OLD	NEW
(Empty)
	1
	juliandoucette 2017/10/13 13:39:55 NIT: Regarding the no-js state of this component: NIT: Regarding the no-js state of this component: 1. You could hide it entirely 2. You could add <a href="#platform-${browser}"></a> around each option a. And align the label or the dropdown top b. And show one option per line; preferably ire 2017/10/17 15:03:33 I think I will hide it entirely. Because each sect Show quoted text On 2017/10/13 13:39:55, juliandoucette wrote: > NIT: Regarding the no-js state of this component: > > 1. You could hide it entirely > 2. You could add <a href="#platform-${browser}"></a> around each option > a. And align the label or the dropdown top > b. And show one option per line; preferably I think I will hide it entirely. Because each section has the classname .platform-${browser}, not the unique ID ire 2017/10/17 15:03:33 Done. Show quoted text On 2017/10/13 13:39:55, juliandoucette wrote: > NIT: Regarding the no-js state of this component: > > 1. You could hide it entirely > 2. You could add <a href="#platform-${browser}"></a> around each option > a. And align the label or the dropdown top > b. And show one option per line; preferably Done. juliandoucette 2017/10/18 15:04:10 NIT: Adding an id (too) would make them deep linka Show quoted text On 2017/10/17 15:03:33, ire wrote: > On 2017/10/13 13:39:55, juliandoucette wrote: > > NIT: Regarding the no-js state of this component: > > > > 1. You could hide it entirely > > 2. You could add <a href="#platform-${browser}"></a> around each option > > a. And align the label or the dropdown top > > b. And show one option per line; preferably > > I think I will hide it entirely. Because each section has the classname > .platform-${browser}, not the unique ID NIT: Adding an id (too) would make them deep linkable without JS. ire 2017/10/20 13:40:46 The issue with adding an id was because of section Show quoted text On 2017/10/18 15:04:10, juliandoucette wrote: > On 2017/10/17 15:03:33, ire wrote: > > On 2017/10/13 13:39:55, juliandoucette wrote: > > > NIT: Regarding the no-js state of this component: > > > > > > 1. You could hide it entirely > > > 2. You could add <a href="#platform-${browser}"></a> around each option > > > a. And align the label or the dropdown top > > > b. And show one option per line; preferably > > > > I think I will hide it entirely. Because each section has the classname > > .platform-${browser}, not the unique ID > > NIT: Adding an id (too) would make them deep linkable without JS. The issue with adding an id was because of sections that apply to multiple platforms. And there are sometimes multiple sections for one platform too.
	2 <label id="browser-select-label">

	3 {{ "Your Browser" \| translate("browser-select-label", "Label") }}:

	4 </label>

	5

	6 <div id="browser-select" class="custom-select custom-select-dropdown">

	7 <button class="custom-select-selected" aria-expanded="false" aria-haspopup="tr ue">

	8 </button>

	9 <ul class="custom-select-options" role="menu" tabindex="-1" aria-labelledby="b rowser-select-label">

	10 {% for browser in browsers %}

	11 <li class="custom-select-option" role="button" tabindex="0" data-value="{{ b rowser.id }}">
	juliandoucette 2017/10/13 13:39:55 NIT/Suggest: role="menuitemradio" aria-checked="fa NIT/Suggest: role="menuitemradio" aria-checked="false" Source: https://www.w3.org/TR/wai-aria-practices-1.1/examples/menubar/menubar-2/menub... ire 2017/10/17 15:03:33 Done. Show quoted text On 2017/10/13 13:39:55, juliandoucette wrote: > NIT/Suggest: role="menuitemradio" aria-checked="false" > > Source: > https://www.w3.org/TR/wai-aria-practices-1.1/examples/menubar/menubar-2/menub... Done.
	12 <img src="/img/png/logo-abp.png" srcset="/img/svg/logo-abp.svg 2x">
	juliandoucette 2017/10/13 13:39:55 Note: Assuming you will replace these icons Note: Assuming you will replace these icons ire 2017/10/17 15:03:33 Yes I mentioned that I am still waiting on the ico Show quoted text On 2017/10/13 13:39:55, juliandoucette wrote: > Note: Assuming you will replace these icons Yes I mentioned that I am still waiting on the icons in a previous comment. juliandoucette 2017/10/18 15:04:10 Acknowledged. Show quoted text On 2017/10/17 15:03:33, ire wrote: > On 2017/10/13 13:39:55, juliandoucette wrote: > > Note: Assuming you will replace these icons > > Yes I mentioned that I am still waiting on the icons in a previous comment. Acknowledged.
	13 {{ browser.name \| translate(browser.id + "-name") }}
	juliandoucette 2017/10/13 13:39:55 Missing (autodetected) label? (See comment js fil Missing (autodetected) label? (See comment js file) ire 2017/10/17 15:03:33 Acknowledged. Show quoted text On 2017/10/13 13:39:55, juliandoucette wrote: > Missing (autodetected) label? > > (See comment js file) Acknowledged.
	14 </li>

	15 {% endfor %}

	16 </ul>

	17 </div>

	18

OLD	NEW

« no previous file with comments | « globals/browsers.py ('k') | static/img/png/arrow-down-icon-secondary.png » ('j') | static/js/main.js » ('J')