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

Delta Between Two Patch Sets: 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

Left Patch Set: Rebased Created Oct. 11, 2017, 5:20 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

« no previous file with change/comment | « includes/adblockplus/remove-a-website-from-the-whitelist.md ('k') | pages/adblockplus/add-a-filter-list.md » ('j') | static/scss/components/_select.scss » ('J')
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Hide Comments ('s')

LEFT	RIGHT
	1 <fieldset>

	2 <label id="browser-select-label">

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

	4 </label>

1	5
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">	6 <div id="browser-select" class="custom-select custom-select-dropdown">

3 {{ "Your Browser" \| translate("browser-select-label", "Label") }}:	7 <button class="custom-select-selected" aria-expanded="false" aria-haspopup=" true">

4 </label>	8 </button>

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

	10 {% for browser in browsers %}

	11 <li class="custom-select-option" role="menuitemradio" aria-checked="false" tabindex="0" data-value="{{ browser.id }}">

	12 <span class="has-pre-icon">

	13 <span class="pre-icon" style="{{ get_inline_bg('logo-'+product_id) }}" ></span>

	14 {{ browser.name \| translate(browser.id + "-name") }}

	15 </span>

	16 </li>

	17 {% endfor %}

	18 </ul>

	19 </div>

	20 </fieldset>

5	21

6 <div id="browser-select" class="custom-select custom-select-dropdown">	22 <script id="browser-select-autodetected" type="text/template">{{ "autodetected" \| translate("browser-select-autodetected") }}</script>

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

LEFT	RIGHT