Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code
Issues
All Issues

Unified Diff: options.html

Issue 6088024630755328: issue 1526 - Implement new options page design for Chrome/Opera/Safari (Closed)
Patch Set: Created Jan. 21, 2015, 6:52 p.m.
Use n/p to move between diff chunks; N/P to move between comments.
Jump to:
View side-by-side diff with in-line comments
Download patch
« locale/en-US/options.json ('K') | « locale/en-US/options.json ('k') | options.js » ('j') | options.js » ('J')
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: options.html
===================================================================
new file mode 100644
--- /dev/null
+++ b/options.html
@@ -0,0 +1,258 @@
+<!DOCTYPE html>
Thomas Greiner 2015/01/22 17:56:30 This page contains a couple of subpages so I'd fin
saroyanm 2015/01/23 18:18:39 Done.
+<!--
+ - This file is part of Adblock Plus <http://adblockplus.org/>,
Thomas Greiner 2015/01/22 17:56:30 Nit: Change to "https"
saroyanm 2015/01/23 18:18:39 Done.
+ - Copyright (C) 2006-2015 Eyeo GmbH
+ -
+ - Adblock Plus is free software: you can redistribute it and/or modify
+ - it under the terms of the GNU General Public License version 3 as
+ - published by the Free Software Foundation.
+ -
+ - Adblock Plus is distributed in the hope that it will be useful,
+ - but WITHOUT ANY WARRANTY; without even the implied warranty of
+ - MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ - GNU General Public License for more details.
+ -
+ - You should have received a copy of the GNU General Public License
+ - along with Adblock Plus. If not, see <http://www.gnu.org/licenses/>.
+ -->
+
+<html>
+ <head>
+ <meta charset="utf-8">
+ <title class="i18n_options_page_title"></title>
+ <link rel="stylesheet" href="skin/options.css">
+ <script type="text/javascript" src="ext/common.js"></script>
Thomas Greiner 2015/01/22 17:56:30 No need to specify the <script> type
saroyanm 2015/01/23 18:18:39 Done. We are using type attribute also in other HT
Thomas Greiner 2015/01/26 18:29:17 No, we usually take care of inconsistencies when s
+ <script type="text/javascript" src="ext/content.js"></script>
+ <script type="text/javascript" src="i18n.js"></script>
+ <script type="text/javascript" src="options.js"></script>
+ </head>
+ <body>
+ <div id="left-sidebar">
+ <div id="fixed-sidebar" class="fixed">
+ <div id="page-title">
+ <p class="i18n_options_page_name"></p>
+ <h1>Adblock <b>Plus</b></h1>
Thomas Greiner 2015/01/22 17:56:30 This text needs to be taken from "locale/en-US/opt
saroyanm 2015/01/23 18:18:39 Done.
+ </div>
+ <ul class="tabs vertical" id="main-navigation-tabs">
+ <li id="tab-general" data-show="content-general" class="active"><span class="i18n_options_tab_general"></span><span class="icon"></span></li>
Thomas Greiner 2015/01/22 17:56:30 Don't put every list item in one single line to im
saroyanm 2015/01/23 18:18:39 Done.
+ <li id="tab-advanced" data-show="content-advanced"><span class="i18n_options_tab_advanced"></span><span class="icon"></span></li>
+ <li id="tab-help" data-show="content-help"><span class="i18n_options_tab_help"></span><span class="icon"></span></li>
+ </ul>
+ <p class="nav-blue">
Thomas Greiner 2015/01/22 17:56:30 Don't include colors in class names because the co
saroyanm 2015/01/23 18:18:39 Done.
+ <span class="i18n_options_version"></span> <span id="version">2.5.10</span>
Thomas Greiner 2015/01/22 17:56:30 Don't hardcode the version number. Request it from
saroyanm 2015/01/23 18:18:39 Done.
+ </p>
+ <ul class="tabs vertical bottom">
+ <li id="tab-share"><span class="i18n_options_tab_share"></span><span class="icon"></span></li>
+ <li id="tab-donate"><span class="i18n_options_tab_donate"></span><span class="icon"></span></li>
+ </ul>
+ </div>
+ </div>
+ <div id="tab-content">
+ <div id="content-wrapper">
+ <div id="modal-background"></div>
+ <div id="content-general">
+ <div>
Thomas Greiner 2015/01/22 17:56:30 You're wrapping everything inside a <div> but don'
saroyanm 2015/01/23 18:18:39 Thanks, indentation was missing it wraps Blocking
+ <h1 class="i18n_options_blocking"></h1>
+ <hr>
Thomas Greiner 2015/01/22 17:56:30 You could use "border-bottom" instead of <hr> here
saroyanm 2015/01/23 18:18:39 Done.
+ <div class="flex-container">
+ <div id="blocking-languages" class="left">
Thomas Greiner 2015/01/22 17:56:30 Don't use "left" or "right" in class names since w
saroyanm 2015/01/23 18:18:39 Done.
+ <p class="option-name"><b class="i18n_options_language_adblocking"></b> <span class="i18n_options_language_title"></span> <a class="i18n_options_readMore dotted" href="#" target="_blank"></a></p>
Thomas Greiner 2015/01/22 17:56:30 Generally, we avoid using HTML elements that conve
saroyanm 2015/01/23 18:18:39 Done.
+ <ul class="table list" id="blocking-languages-table"></ul>
+ <hr>
+ <div class="controls">
+ <div class="button" id="add-website-language" data-show="blocking-languages-modal">
Thomas Greiner 2015/01/22 17:56:30 It'd be great if you could use standard HTML eleme
saroyanm 2015/01/23 18:18:39 Done.
+ <span class="icon icon-add"></span><span class="i18n_options_language_add text-blue"></span>
+ </div>
+ </div>
+ </div>
+ <div id="further-blocking">
+ <p class="option-name"><b class="i18n_options_furtherBlocking_title"></b> <a class="i18n_options_readMore dotted" href="#" target="_blank"></a></p>
Thomas Greiner 2015/01/22 17:56:30 Again, don't use implementation specific details l
saroyanm 2015/01/23 18:18:39 Done.
+ <ul class="table list" id="further-list-table"></ul>
+ <hr>
+ <div class="controls">
+ <div class="button" id="add-blocking-list" data-show="further-blocking-modal">
+ <span class="icon icon-add"></span><span class="i18n_options_furtherBlocking_add text-blue"><span>
+ </div>
+ </div>
+ </div>
+ </div>
+ </div>
+
+ <div>
+ <h1 class="i18n_options_exception"></h1>
+ <hr>
+ <div class="flex-container">
+ <div class="left" id="acceptable-ads">
+ <p class="option-name"><b class="i18n_options_acceptableAds_title"></b> <a class="i18n_options_readMore dotted" href="#" target="_blank"></a></p>
+ <ul class="table list">
+ <li><input type='checkbox'/><label id="allow-whitelist-cb"></label><span>Allow some non-intrusive advertising</span></li>
Thomas Greiner 2015/01/22 17:56:30 Use " instead of ' for consistency
Thomas Greiner 2015/01/22 17:56:30 Actually, the <span> should be the label for the c
saroyanm 2015/01/23 18:18:39 Done.
saroyanm 2015/01/23 18:18:39 Yes for now, It make sense to change with span, be
Thomas Greiner 2015/01/26 18:29:17 That's not what I meant. I thought more of somethi
+ </ul>
+ </div>
+ <div id="whitelisting">
+ <p class="option-name"><b class="i18n_options_whitelisted_title"></b> <a class="i18n_options_readMore dotted" href="#" target="_blank"></a></p>
+ <ul class="table list" id="whitelisting-table"></ul>
+ <hr>
+ <div class="controls">
+ <span class="icon icon-add" id="whitelisting-add-icon"></span>
+ <input type="text" id="whitelisting-textbox" />
+ <span class="icon icon-enter" id="whitelisting-enter-icon"></span>
+ </div>
+ <div style="display: flex;">
Thomas Greiner 2015/01/22 17:56:30 All styles should be inside a CSS file to separate
saroyanm 2015/01/23 18:18:39 Done.
+ <button class="addbtn" id="whitelisting-add-btn">+<span class="i18n_options_btn_add"></span></button>
Thomas Greiner 2015/01/22 17:56:30 Why not "button-add"? That should be more consiste
saroyanm 2015/01/23 18:18:39 Done.
+ <span style="flex: 1;"></span>
+ <button class="i18n_options_btn_cancel cancelbtn" id="whitelisting-cancel-btn"></button>
+ </div>
+ </div>
+ </div>
+ </div>
+ </div>
+ <div id="content-advanced">
+ <div>
+ <h1 class="i18n_options_tweaks"></h1>
+ <a class="i18n_options_readMore dotted" href="#"></a>
+ <hr />
+ <ul class="table" style="width: auto;">
+ <li>
+ <input type='checkbox' id="easylist"/><label for="easylist"></label><span class="i18n_options_tweaks_blockElement"></span> <a class="i18n_options_tweaks_blockElement_highlight dotted" href="#" target="_blank"></a></li>
+ </ul>
+ </div>
+ <div>
+ <h1 class="i18n_options_blockingList"></h1>
+ <a class="i18n_options_readMore dotted" href="#" target="_blank"></a>
+ <hr />
+ <ul class="tabs horizontal" id="blocking-list-tabs">
+ <li class="i18n_options_tab_overview active" data-show="blocking-list-overview"></li><li class="i18n_options_tab_ownList" data-show="blocking-list-own"></li>
+ </ul>
+ <div id="blocking-list">
+ <div id="blocking-list-overview">
+ <ul class="table cols" style="width: auto;">
+ <li class="col-name"><span class="i18n_options_tableCol_name"></span><span class="i18n_options_tableCol_description"></span><span class="i18n_options_tableCol_date"></span></li>
+ <li><input type='checkbox' id="easylist"/><label for="easylist"></label><span>Easylist</span><span>Adblocking english sites</span><span>15 March 14 - 10:31</span></li>
+ <li><input type='checkbox' id="easylist+de"/><label for="easylist+de"></label><span>Easylist Germany + Easylist</span><span>Adblocking english + german sites</span><span>15 March 14 - 10:31</span></li>
+ <li><input type='checkbox' id="annoyance-fb"/><label for="annoyance-fb"></label><span>Facebook annoyance blocker</span><span>Blocks Facebook annoyances</span><span>15 March 14 - 10:31</span></li>
+ <li><input type='checkbox' id="annoyance-youtube"/><label for="annoyance-youtube"></label><span>Facebook annoyance blocker</span><span>Blocks Facebook annoyances</span><span>15 March 14 - 10:31</span></li>
+ <li><input type='checkbox' id="own-list"/><label for="own-list"></label><span>Own blocking list</span><span>Your own blocking list</span><span><a href="#">edit your blocking list</a></span></li>
+ </ul>
+ <hr class=""/>
+ <div class="controls">
+ <div class="button">
+ <span class="icon icon-add"></span><span class="i18n_options_blockingList_add text-blue"></span>
+ </div>
+ <div class="button">
+ <span class="icon icon-update"></span><span class="i18n_options_blockingList_update text-blue"></span>
+ </div>
+ </div>
+ </div>
+ <div id="blocking-list-own">
+ <p class="i18n_options_blockingRules"></p>
+ <ul class="table list">
+ <li><span>zap2it.com##.zc-station-position</span></li>
+ <li><span>downturk.net##.zippo</span></li>
+ <li><span>yahoo.com##.y708-promo-middle</span></li>
+ <li><span>reflector.com##.yahooboss</span></li>
+ <li><span>yardbarker.com##.yard_leader</span></li>
+ <li><span>espn.co.uk##.will_hill</span></li>
+ <li><span>listverse.com##.wiki</span></li>
+ <li><span>planet5d.com##.wp-image-1573</span></li>
+ <li><span>buzzinn.net##.wpn_finner</span></li>
+ <li><span>talkers.com##.wpss_slideshow</span></li>
+ </ul>
+ <div class="controls" style="margin: 0px;">
+ <input type="text" placeholder="add your blocking rule here"/>
+ <div class="input-control">
+ <span class="input-separator"></span>
+ <span class="i18n_options_btn_add input-btn-text"></span>
+ <span class="icon icon-enter-blue"></span>
+ </div>
+ </div>
+ <div class="controls button">
+ <span class="icon icon-edit"></span>
+ <span class="i18n_options_blockingRules_edit text-blue"></span>
+ </div>
+ </div>
+ </div>
+ </div>
+ </div>
+ <div id="content-help">
+ <h1>1. <span class="i18n_options_faq"></span></h1>
Thomas Greiner 2015/01/22 17:56:30 No need to do manual enumeration. Just use <ol> or
saroyanm 2015/01/23 18:18:39 Done.
+ <hr />
+ <p class="i18n_options_faq_description"></p>
+ <p>
+ <span class="icon icon-arrow"></span>
Thomas Greiner 2015/01/22 17:56:30 This can be added using the "::before" pseudo elem
saroyanm 2015/01/23 18:18:39 Done.
+ <a class="i18n_options_faq_link text-blue" href="#" target="_blank"></a>
+ </p>
+ <h1>2. <span class="i18n_options_forum"></span></h1>
+ <hr />
+ <p class="i18n_options_forum_description"></p>
+ <p>
+ <span class="icon icon-arrow"></span>
+ <a class="i18n_options_forum_link text-blue" href="#" target="_blank"></a>
+ </p>
+ <h1>3. <span class="i18n_options_media"></span></h1>
+ <hr />
+ <p class="i18n_options_media_description"></p>
+ <p>
+ <span class="icon icon-arrow"></span>
+ <a class="text-blue" href="#" target="_blank">Twitter</a>
+ <span class="icon icon-arrow"></span>
+ <a class="text-blue" href="#" target="_blank">Facebook</a>
+ <span class="icon icon-arrow"></span>
+ <a class="text-blue" href="#" target="_blank">Google+</a>
+ </p>
+ </div>
+ </div>
+ </div>
+ <div id="modal" style="display: block;">
+ <div>
Thomas Greiner 2015/01/22 17:56:30 We use two spaces for indentation.
saroyanm 2015/01/23 18:18:39 Done.
+ <div class="header">
Thomas Greiner 2015/01/22 17:56:30 Use the <header> element instead to stick to stand
saroyanm 2015/01/23 18:18:39 Done.
+ <div>
+ <div>
+ <span id="modal-title"></span>
Thomas Greiner 2015/01/22 17:56:30 This structure is quite confusing and looks somewh
saroyanm 2015/01/23 18:18:39 Thanks, should be better now.
+ </div>
+ <div class="close-wrapper">
+ <span class="separator"></span>
Thomas Greiner 2015/01/22 17:56:30 I guess this can be achieved using a border.
saroyanm 2015/01/23 18:18:39 Done.
+ <div class="button" id="modal-close">
+ <span class="icon icon-close"></span>
+ <span class="i18n_options_close close"></span>
+ </div>
+ </div>
+ </div>
+ </div>
+ <div class="content" id="modal-content">
Thomas Greiner 2015/01/22 17:56:30 What about the dialog for "abp:subscribe" links? I
saroyanm 2015/01/23 18:18:39 I thought it should be same dialog. Can you pleas
Thomas Greiner 2015/01/26 18:29:17 I asked Sven to attach it to the issue description
+ <div style="display: none;" id="blocking-languages-modal" data-title="options_modal_language_title">
Thomas Greiner 2015/01/22 17:56:30 What do you need the "data-title" attribute here f
saroyanm 2015/01/23 18:18:39 I'm using that attribute to change the title of mo
Thomas Greiner 2015/01/26 18:29:17 You could achieve the same effect using a CSS clas
+ <div>
+ <h3 class="i18n_options_modal_language_added"></h3>
+ <ul class="table list" id="blocking-languages-modal-table"></ul>
+ </div>
+ <div id="other-language">
+ <h3 class="i18n_options_modal_language_other"></h3>
+ <div>
+ <input type="text" placeholder="find language" id="find-language" />
+ </div>
+ <ul class="table list" id="all-lang-table"></ul>
+ </div>
+ </div>
+ <div style="display: none;" id="further-blocking-modal" data-title="options_modal_blocklist_title">
+ <div>
+ <h3 class="i18n_options_modal_blocklist_subscription_title"></h3>
+ <div>
+ <input id="blockingList-textbox" type="text" placeholder="www.yourURL.com/blockinglist.txt" />
+ </div>
+ <div class="btn-wrapper" id="import-blockingList-btn">
+ <span class="icon icon-arrow"></span>
+ <span class="i18n_options_modal_blocklist_import text-blue"></span>
+ </div>
+ </div>
+ <div>
+ <h3 class="i18n_options_modal_edit_own_list"></h3>
+ <div class="btn-wrapper" id="edit-ownBlockingList-btn">
+ <span class="icon icon-arrow"></span>
+ <span class="i18n_options_modal_create_own_list text-blue"></span>
+ </div>
+ </div>
+ </div>
+ </div>
+ </div>
+ </div>
+ </body>
+</html>
« locale/en-US/options.json ('K') | « locale/en-US/options.json ('k') | options.js » ('j') | options.js » ('J')

Powered by Google App Engine
This is Rietveld