new-options.html - Issue 29338983: issue 3741 - Add "remove" option to list items in new options page

Side by Side Diff: new-options.html

Issue 29338983: issue 3741 - Add "remove" option to list items in new options page (Closed)

Patch Set: Rebase to changeset #83 Created May 10, 2016, 2:17 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
1 <!DOCTYPE html>	1 <!DOCTYPE html>

2 <!--	2 <!--

3 - This file is part of Adblock Plus <https://adblockplus.org/>,	3 - This file is part of Adblock Plus <https://adblockplus.org/>,

4 - Copyright (C) 2006-2016 Eyeo GmbH	4 - Copyright (C) 2006-2016 Eyeo GmbH

5 -	5 -

6 - Adblock Plus is free software: you can redistribute it and/or modify	6 - Adblock Plus is free software: you can redistribute it and/or modify

7 - it under the terms of the GNU General Public License version 3 as	7 - it under the terms of the GNU General Public License version 3 as

8 - published by the Free Software Foundation.	8 - published by the Free Software Foundation.

9 -	9 -

10 - Adblock Plus is distributed in the hope that it will be useful,	10 - Adblock Plus is distributed in the hope that it will be useful,

(...skipping 80 matching lines...) Expand 10 before \| Expand all \| Expand 10 after Loading...
91 <div class="notes">	91 <div class="notes">

92 <p class="i18n_options_language_title_tooltip_notes_1">< /p>	92 <p class="i18n_options_language_title_tooltip_notes_1">< /p>

93 <p class="i18n_options_language_title_tooltip_notes_2">< /p>	93 <p class="i18n_options_language_title_tooltip_notes_2">< /p>

94 </div>	94 </div>

95 </div>	95 </div>

96 </div>	96 </div>

97 </div>	97 </div>

98 <ul id="blocking-languages-table" class="table list">	98 <ul id="blocking-languages-table" class="table list">

99 <template>	99 <template>

100 <button role="checkbox" class="control"></button>	100 <button role="checkbox" class="control"></button>

101 <label class="display"></label>	101 <span class="display"></span>
	Thomas Greiner 2016/05/23 12:43:25 What's the reason for changing this to `<span>` he What's the reason for changing this to `<span>` here? Seems like it was done by accident since the other `<label>` elements are unchanged. saroyanm 2016/06/08 15:21:06 I think, missed this during the rebase, thanks. Show quoted text On 2016/05/23 12:43:25, Thomas Greiner wrote: > What's the reason for changing this to `<span>` here? Seems like it was done by > accident since the other `<label>` elements are unchanged. I think, missed this during the rebase, thanks.
	102 <button data-action="remove-subscription" class="delete cont rol"></button>
	Thomas Greiner 2016/05/23 12:43:25 This button's accessibility needs to be improved s This button's accessibility needs to be improved since it's unclear to visually impaired users what this button does. At the same time we may also want to clarify the action of the checkbox since its label might not be sufficient anymore due to there being two actions now. So what I could imagine is something along those lines: <li id="item-42" role="section"> <button role="checkbox" class="control" title="Toggle subscription"></button> <label for="item-42">EasyList</label> <button class="control delete" data-action="remove-subscription" title="Remove subscription"></button> </li> If you want, this can be tackled in a separate ticket though. saroyanm 2016/06/08 15:21:06 Updated: I tested the suggestion you've made with Show quoted text On 2016/05/23 12:43:25, Thomas Greiner wrote: > This button's accessibility needs to be improved since it's unclear to visually > impaired users what this button does. At the same time we may also want to > clarify the action of the checkbox since its label might not be sufficient > anymore due to there being two actions now. > > So what I could imagine is something along those lines: > > <li id="item-42" role="section"> > <button role="checkbox" class="control" title="Toggle subscription"></button> > <label for="item-42">EasyList</label> > <button class="control delete" data-action="remove-subscription" title="Remove > subscription"></button> > </li> > > If you want, this can be tackled in a separate ticket though. Updated: I tested the suggestion you've made with the screen recorder, apparently it's still not obvious what element should toggle/remove the buttons, so I've added "aria-labelledby" to both of the elements to point the listElement, but I had to add "label" attribute to the list element as well to make that connection work, anyway I can't understand the value of role element and using for on "label" element. Also the only advantage I can see after testing with current approach that it will be possible to setup label on parent element regardless of the label, so for example we can have "Filter list: German".
102 </template>	103 </template>

103 </ul>	104 </ul>

104 <div class="controls">	105 <div class="controls">

105 <button data-action="open-dialog" data-dialog="language">	106 <button data-action="open-dialog" data-dialog="language">

106 <span class="icon icon-add"></span>	107 <span class="icon icon-add"></span>

107 <span class="i18n_options_language_add"></span>	108 <span class="i18n_options_language_add"></span>

108 </button>	109 </button>

109 </div>	110 </div>

110 </div>	111 </div>

111 <div id="further-blocking">	112 <div id="further-blocking">

112 <div class="option-name">	113 <div class="option-name">

113 <strong class="i18n_options_furtherBlocking_title"></strong>	114 <strong class="i18n_options_furtherBlocking_title"></strong>

114 <div class="tooltip">	115 <div class="tooltip">

115 <span class="i18n_options_readMore"></span>	116 <span class="i18n_options_readMore"></span>

116 <div role="tooltip" class="flip-vertical">	117 <div role="tooltip" class="flip-vertical">

117 <img src="skin/tooltips/more.png" alt="" />	118 <img src="skin/tooltips/more.png" alt="" />

118 <p class="i18n_options_furtherBlocking_title_tooltip"></p>	119 <p class="i18n_options_furtherBlocking_title_tooltip"></p>

119 </div>	120 </div>

120 </div>	121 </div>

121 </div>	122 </div>

122 <div id="custom-wrapper">	123 <div id="custom-wrapper">

123 <ul id="recommend-list-table" class="table list">	124 <ul id="recommend-list-table" class="table list">

124 <template>	125 <template>

125 <button role="checkbox" class="control"></button>	126 <button role="checkbox" class="control"></button>

126 <label class="display"></label>	127 <label class="display"></label>

127 <span class="i18n_options_popular popular"></span>

128 </template>	128 </template>

129 </ul>	129 </ul>

130 <ul id="custom-list-table" class="table list">	130 <ul id="custom-list-table" class="table list">

131 <template>	131 <template>

132 <button role="checkbox" class="control"></button>	132 <button role="checkbox" class="control"></button>

133 <label class="display"></label>	133 <label class="display"></label>

	134 <button data-action="remove-subscription" class="delete co ntrol"></button>

134 </template>	135 </template>

135 </ul>	136 </ul>

136 </div>	137 </div>

137 <div class="controls">	138 <div class="controls">

138 <button data-action="open-dialog" data-dialog="custom">	139 <button data-action="open-dialog" data-dialog="custom">

139 <span class="icon icon-add"></span>	140 <span class="icon icon-add"></span>

140 <span class="i18n_options_furtherBlocking_add"><span>	141 <span class="i18n_options_furtherBlocking_add"><span>

141 </button>	142 </button>

142 </div>	143 </div>

143 </div>	144 </div>

(...skipping 32 matching lines...) Expand 10 before \| Expand all \| Expand 10 after Loading...
176 <p class="i18n_options_whitelisted_title_tooltip_1"></p>	177 <p class="i18n_options_whitelisted_title_tooltip_1"></p>

177 <p class="i18n_options_whitelisted_title_tooltip_2"></p>	178 <p class="i18n_options_whitelisted_title_tooltip_2"></p>

178 <div class="notes">	179 <div class="notes">

179 <p class="i18n_options_whitelisted_title_tooltip_notes"> </p>	180 <p class="i18n_options_whitelisted_title_tooltip_notes"> </p>

180 </div>	181 </div>

181 </div>	182 </div>

182 </div>	183 </div>

183 </div>	184 </div>

184 <ul id="whitelisting-table" class="table list">	185 <ul id="whitelisting-table" class="table list">

185 <template>	186 <template>

	187 <label class="display"></label>

186 <button class="delete control"></button>	188 <button class="delete control"></button>

187 <label class="display"></label>

188 </template>	189 </template>

189 </ul>	190 </ul>

190 <div class="controls">	191 <div class="controls">

191 <button data-action="edit-domain-exception">	192 <button data-action="edit-domain-exception">

192 <span class="icon icon-add"></span>	193 <span class="icon icon-add"></span>

193 <span class="i18n_options_whitelisted_add"></span>	194 <span class="i18n_options_whitelisted_add"></span>

194 </button>	195 </button>

195 <div>	196 <div>

196 <span class="icon icon-add" data-action="add-domain-exceptio n"></span>	197 <span class="icon icon-add" data-action="add-domain-exceptio n"></span>

197 <input type="text" id="whitelisting-textbox" placeholder="ww w.example.com" />	198 <input type="text" id="whitelisting-textbox" placeholder="ww w.example.com" />

(...skipping 235 matching lines...) Expand 10 before \| Expand all \| Expand 10 after Loading...
433 <div class="url"></div>	434 <div class="url"></div>

434 <button class="i18n_options_dialog_predefined_confirm default-focus" data-action="add-predefined-subscription"></button>	435 <button class="i18n_options_dialog_predefined_confirm default-focus" data-action="add-predefined-subscription"></button>

435 </div>	436 </div>

436 </div>	437 </div>

437 </div>	438 </div>

438 <!-- Placeholder element to determine when to wrap focus around -->	439 <!-- Placeholder element to determine when to wrap focus around -->

439 <span class="focus-last" tabindex="0"></span>	440 <span class="focus-last" tabindex="0"></span>

440 </div>	441 </div>

441 </body>	442 </body>

442 </html>	443 </html>

OLD	NEW

« no previous file with comments | « locale/en-US/new-options.json ('k') | new-options.js » ('j') | skin/new-options.css » ('J')