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

Issue 29337807: Issue 3097 - Refactored subscriptions list table on adblockplus.org/en/subscriptions (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
3 years, 12 months ago by juliandoucette
Modified:
3 years, 6 months ago
CC:
kzar
Visibility:
Public.

Description

Changed subscriptions list table into a hierarchical list for accessibility and to support multi-level supplemented lists.

Patch Set 1 : Changed table into unordered list, refactored list markup, refactored main css and templates to support multiple footer elements #

Total comments: 15

Patch Set 2 : Refactored markup & styles, added rtl styles, and fixed typo. #

Total comments: 10

Patch Set 3 : Changed footer-main class to id, removed .screen-reader-text class #

Total comments: 2

Patch Set 4 : Removed extra whitespace #

Unified diffs Side-by-side diffs Delta from patch set Stats (+159 lines, -83 lines) Patch
M includes/subscriptionList.tmpl View 1 2 3 1 chunk +67 lines, -49 lines 0 comments Download
M pages/subscriptions.tmpl View 1 2 1 chunk +69 lines, -12 lines 0 comments Download
M static/css/main.css View 1 2 1 chunk +5 lines, -4 lines 0 comments Download
M static/css/main-desktop.css View 1 2 1 chunk +8 lines, -8 lines 0 comments Download
M static/css/main-mobile.css View 1 2 2 chunks +8 lines, -8 lines 0 comments Download
M templates/default.tmpl View 1 2 1 chunk +1 line, -1 line 0 comments Download
M templates/simple.tmpl View 1 2 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 10
juliandoucette
3 years, 12 months ago (2016-02-27 16:37:06 UTC) #1
Thomas Greiner
https://codereview.adblockplus.org/29337807/diff/29337808/includes/subscriptionList.tmpl File includes/subscriptionList.tmpl (right): https://codereview.adblockplus.org/29337807/diff/29337808/includes/subscriptionList.tmpl#newcode25 includes/subscriptionList.tmpl:25: <span class="sr-only">{{ "Specialization"|translate("specialization") }}:</span> Detail: Not sure what "sr-only" ...
3 years, 7 months ago (2016-07-15 16:07:55 UTC) #2
juliandoucette
Patch submitted. https://codereview.adblockplus.org/29337807/diff/29337808/includes/subscriptionList.tmpl File includes/subscriptionList.tmpl (right): https://codereview.adblockplus.org/29337807/diff/29337808/includes/subscriptionList.tmpl#newcode25 includes/subscriptionList.tmpl:25: <span class="sr-only">{{ "Specialization"|translate("specialization") }}:</span> On 2016/07/15 16:07:53, ...
3 years, 7 months ago (2016-07-19 22:29:18 UTC) #3
Thomas Greiner
https://codereview.adblockplus.org/29337807/diff/29337808/static/css/main.css File static/css/main.css (left): https://codereview.adblockplus.org/29337807/diff/29337808/static/css/main.css#oldcode256 static/css/main.css:256: #content h1 On 2016/07/19 22:29:17, juliandoucette wrote: > On ...
3 years, 7 months ago (2016-07-27 09:42:10 UTC) #4
juliandoucette
One question in the comments. https://codereview.adblockplus.org/29337807/diff/29348048/pages/subscriptions.tmpl File pages/subscriptions.tmpl (right): https://codereview.adblockplus.org/29337807/diff/29348048/pages/subscriptions.tmpl#newcode23 pages/subscriptions.tmpl:23: clip: rect(1px, 1px, 1px, ...
3 years, 7 months ago (2016-07-27 12:11:01 UTC) #5
Thomas Greiner
https://codereview.adblockplus.org/29337807/diff/29348048/pages/subscriptions.tmpl File pages/subscriptions.tmpl (right): https://codereview.adblockplus.org/29337807/diff/29348048/pages/subscriptions.tmpl#newcode23 pages/subscriptions.tmpl:23: clip: rect(1px, 1px, 1px, 1px); On 2016/07/27 12:11:01, juliandoucette ...
3 years, 7 months ago (2016-07-27 13:25:49 UTC) #6
juliandoucette
https://codereview.adblockplus.org/29337807/diff/29348048/pages/subscriptions.tmpl File pages/subscriptions.tmpl (right): https://codereview.adblockplus.org/29337807/diff/29348048/pages/subscriptions.tmpl#newcode23 pages/subscriptions.tmpl:23: clip: rect(1px, 1px, 1px, 1px); On 2016/07/27 13:25:48, Thomas ...
3 years, 7 months ago (2016-07-28 21:34:02 UTC) #7
Thomas Greiner
Apart from the remaining coding style issue: LGTM https://codereview.adblockplus.org/29337807/diff/29348833/includes/subscriptionList.tmpl File includes/subscriptionList.tmpl (right): https://codereview.adblockplus.org/29337807/diff/29348833/includes/subscriptionList.tmpl#newcode25 includes/subscriptionList.tmpl:25: {{ ...
3 years, 7 months ago (2016-07-29 10:07:39 UTC) #8
juliandoucette
https://codereview.adblockplus.org/29337807/diff/29348833/includes/subscriptionList.tmpl File includes/subscriptionList.tmpl (right): https://codereview.adblockplus.org/29337807/diff/29348833/includes/subscriptionList.tmpl#newcode25 includes/subscriptionList.tmpl:25: {{ "Specialization"|translate("specialization") }}: On 2016/07/29 10:07:38, Thomas Greiner wrote: ...
3 years, 7 months ago (2016-07-29 11:01:36 UTC) #9
juliandoucette
3 years, 6 months ago (2016-08-16 16:56:01 UTC) #10

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