Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(35)

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

Can't Edit
Can't Publish+Mail
Start Review
Created:
4 years, 2 months ago by saroyanm
Modified:
3 years, 11 months ago
Reviewers:
Thomas Greiner
Visibility:
Public.

Description

issue 3741 - Add "remove" option to list items in new options page

Patch Set 1 : #

Total comments: 7

Patch Set 2 : Rebase to changeset #83 #

Total comments: 6

Patch Set 3 : Impoved accessibility, removed onclick assignments from the collections #

Total comments: 3

Patch Set 4 : Added title attribute to whitelisting and custom list table #

Patch Set 5 : Rebaseed to changeset 84 #

Total comments: 10

Patch Set 6 : Translated title attributes #

Total comments: 2

Patch Set 7 : #

Total comments: 2

Patch Set 8 : Removed aria-labelledby #

Total comments: 6

Patch Set 9 : Small fix #

Patch Set 10 : Removed label's "for" attribute, added "aria-lable" to addItem #

Unified diffs Side-by-side diffs Delta from patch set Stats (+74 lines, -103 lines) Patch
M locale/en-US/new-options.json View 1 2 3 4 5 6 2 chunks +8 lines, -4 lines 0 comments Download
M new-options.html View 1 2 3 4 5 6 6 chunks +9 lines, -8 lines 0 comments Download
M new-options.js View 1 2 3 4 5 6 7 8 9 9 chunks +46 lines, -73 lines 0 comments Download
M skin/new-options.css View 1 2 3 4 5 chunks +11 lines, -18 lines 0 comments Download
M skin/options-sprite.png View 1 2 5 Binary file 0 comments Download

Messages

Total messages: 16
saroyanm
Thomas can you please have a look when you have time ? https://codereview.adblockplus.org/29338983/diff/29338996/options.js File options.js ...
4 years, 2 months ago (2016-03-23 16:34:38 UTC) #1
saroyanm
https://codereview.adblockplus.org/29338983/diff/29338996/options.js File options.js (left): https://codereview.adblockplus.org/29338983/diff/29338996/options.js#oldcode363 options.js:363: if (change[i].name == "disabled") On 2016/03/23 16:34:38, saroyanm wrote: ...
4 years ago (2016-05-10 14:26:16 UTC) #2
Thomas Greiner
https://codereview.adblockplus.org/29338983/diff/29338996/options.js File options.js (right): https://codereview.adblockplus.org/29338983/diff/29338996/options.js#newcode305 options.js:305: onClick: toggleDisableSubscription On 2016/03/23 16:34:38, saroyanm wrote: > I ...
4 years ago (2016-05-23 12:43:26 UTC) #3
saroyanm
https://codereview.adblockplus.org/29338983/diff/29338996/options.js File options.js (right): https://codereview.adblockplus.org/29338983/diff/29338996/options.js#newcode305 options.js:305: onClick: toggleDisableSubscription On 2016/05/23 12:43:25, Thomas Greiner wrote: > ...
3 years, 12 months ago (2016-06-08 15:21:07 UTC) #4
Thomas Greiner
https://codereview.adblockplus.org/29338983/diff/29345651/new-options.js File new-options.js (right): https://codereview.adblockplus.org/29338983/diff/29345651/new-options.js#newcode184 new-options.js:184: if (title) On 2016/06/08 15:21:07, saroyanm wrote: > I ...
3 years, 11 months ago (2016-06-16 10:42:23 UTC) #5
saroyanm
https://codereview.adblockplus.org/29338983/diff/29345651/new-options.js File new-options.js (right): https://codereview.adblockplus.org/29338983/diff/29345651/new-options.js#newcode184 new-options.js:184: if (title) On 2016/06/16 10:42:22, Thomas Greiner wrote: > ...
3 years, 11 months ago (2016-06-16 18:22:59 UTC) #6
Thomas Greiner
Just some details left. https://codereview.adblockplus.org/29338983/diff/29345746/new-options.js File new-options.js (right): https://codereview.adblockplus.org/29338983/diff/29345746/new-options.js#newcode188 new-options.js:188: element.setAttribute("label", title); On 2016/06/16 18:22:59, ...
3 years, 11 months ago (2016-06-17 14:07:51 UTC) #7
saroyanm
https://codereview.adblockplus.org/29338983/diff/29345746/new-options.js File new-options.js (right): https://codereview.adblockplus.org/29338983/diff/29345746/new-options.js#newcode188 new-options.js:188: element.setAttribute("label", title); On 2016/06/17 14:07:50, Thomas Greiner wrote: > ...
3 years, 11 months ago (2016-06-21 13:29:57 UTC) #8
Thomas Greiner
https://codereview.adblockplus.org/29338983/diff/29345746/new-options.js File new-options.js (right): https://codereview.adblockplus.org/29338983/diff/29345746/new-options.js#newcode188 new-options.js:188: element.setAttribute("label", title); On 2016/06/21 13:29:56, saroyanm wrote: > On ...
3 years, 11 months ago (2016-06-22 10:26:14 UTC) #9
saroyanm
https://codereview.adblockplus.org/29338983/diff/29345746/new-options.js File new-options.js (right): https://codereview.adblockplus.org/29338983/diff/29345746/new-options.js#newcode188 new-options.js:188: element.setAttribute("label", title); On 2016/06/22 10:26:13, Thomas Greiner wrote: > ...
3 years, 11 months ago (2016-06-22 13:46:17 UTC) #10
Thomas Greiner
https://codereview.adblockplus.org/29338983/diff/29346956/new-options.js File new-options.js (right): https://codereview.adblockplus.org/29338983/diff/29346956/new-options.js#newcode122 new-options.js:122: var titleValue = getMessage(controls[k].getAttribute("title")); Detail: Please keep this in ...
3 years, 11 months ago (2016-06-22 15:49:47 UTC) #11
saroyanm
https://codereview.adblockplus.org/29338983/diff/29346956/new-options.js File new-options.js (right): https://codereview.adblockplus.org/29338983/diff/29346956/new-options.js#newcode122 new-options.js:122: var titleValue = getMessage(controls[k].getAttribute("title")); On 2016/06/22 15:49:46, Thomas Greiner ...
3 years, 11 months ago (2016-06-23 15:04:24 UTC) #12
Thomas Greiner
https://codereview.adblockplus.org/29338983/diff/29346956/new-options.js File new-options.js (right): https://codereview.adblockplus.org/29338983/diff/29346956/new-options.js#newcode192 new-options.js:192: element.setAttribute("aria-label", title); On 2016/06/23 15:04:23, saroyanm wrote: > On ...
3 years, 11 months ago (2016-06-24 15:22:12 UTC) #13
saroyanm
https://codereview.adblockplus.org/29338983/diff/29346956/new-options.js File new-options.js (right): https://codereview.adblockplus.org/29338983/diff/29346956/new-options.js#newcode192 new-options.js:192: element.setAttribute("aria-label", title); On 2016/06/24 15:22:10, Thomas Greiner wrote: > ...
3 years, 11 months ago (2016-06-24 15:54:52 UTC) #14
saroyanm
Thomas can please check the last patch. I've removed the "for" attribute from the <label> ...
3 years, 11 months ago (2016-06-24 16:41:10 UTC) #15
Thomas Greiner
3 years, 11 months ago (2016-06-24 17:03:39 UTC) #16
LGTM
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 87257f5