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

Delta Between Two Patch Sets: static/scss/_navbar.scss

Issue 29587584: Issue 5635 - Implement website-default navbar component (Closed) Base URL: https://hg.adblockplus.org/website-defaults
Left Patch Set: Add navigation <ul> links Created Nov. 10, 2017, 10:22 a.m.
Right Patch Set: Rebase Created Jan. 4, 2018, 7:40 a.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 | « static/js/navbar.js ('k') | static/scss/_variables.scss » ('j') | no next file with change/comment »
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
LEFTRIGHT
1 // This file is part of help.eyeo.com. 1 // This file is part of help.eyeo.com.
2 // Copyright (C) 2017 Eyeo GmbH 2 // Copyright (C) 2017 Eyeo GmbH
3 // 3 //
4 // help.eyeo.com is free software: you can redistribute it and/or modify 4 // help.eyeo.com is free software: you can redistribute it and/or modify
5 // it under the terms of the GNU General Public License as published by 5 // it under the terms of the GNU General Public License as published by
6 // the Free Software Foundation, either version 3 of the License, or 6 // the Free Software Foundation, either version 3 of the License, or
7 // (at your option) any later version. 7 // (at your option) any later version.
8 // 8 //
9 // help.eyeo.com is distributed in the hope that it will be useful, 9 // help.eyeo.com is distributed in the hope that it will be useful,
10 // but WITHOUT ANY WARRANTY; without even the implied warranty of 10 // but WITHOUT ANY WARRANTY; without even the implied warranty of
11 // MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the 11 // MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
12 // GNU General Public License for more details. 12 // GNU General Public License for more details.
13 // 13 //
14 // You should have received a copy of the GNU General Public License 14 // You should have received a copy of the GNU General Public License
15 // along with help.eyeo.com. If not, see <http://www.gnu.org/licenses/>. 15 // along with help.eyeo.com. If not, see <http://www.gnu.org/licenses/>.
16 16
17 /******************************************************************************* 17 /*******************************************************************************
18 * Navbar component 18 * Navbar component
19 ******************************************************************************/ 19 ******************************************************************************/
20 20
21 .navbar 21 .navbar
22 { 22 {
23 @extend .clearfix; 23 @extend .clearfix;
24 } 24 }
25 25
26 .toggle-navbar-collapse, 26 /* .navbar .navbar-container
27 .navbar-collapse 27 ****************************************************************************/
28
29 .navbar-container
28 { 30 {
29 padding-top: $navbar-padding-v + ($navbar-padding-v / 2); 31 @if ($add-horizontal-navbar-padding)
juliandoucette 2017/11/10 11:42:52 I think that this assumes that .toggle-navbar-coll
ire 2017/11/13 17:57:42 You're right. The .toggle-navbar-collapse should a
30 padding-bottom: $navbar-padding-v + ($navbar-padding-v / 2); 32 {
33 @extend .container;
34 }
35 @else
36 {
37 @include container($container-width, 0px);
38 }
31 } 39 }
32 40
33 /* .navbar .navbar-branding 41 /* .navbar .navbar-branding
34 ****************************************************************************/ 42 ****************************************************************************/
juliandoucette 2017/11/10 11:42:52 NIT: There is supposed to be one space before the
ire 2017/11/13 17:57:41 Done.
35 43
36 .navbar-branding 44 .navbar-branding
juliandoucette 2017/11/10 11:42:53 NIT/Suggest: Add padding-x to the navbar-branding
juliandoucette 2017/11/10 11:42:53 Suggest: Add a hover/active/focus background color
37 { 45 {
38 padding-top: $navbar-padding-v; 46 padding-top: $navbar-padding + ($navbar-padding / 4);
39 padding-bottom: $navbar-padding-v; 47 padding-bottom: $navbar-padding + ($navbar-padding / 4);
48 line-height: 1;
49
50 @if ($add-horizontal-navbar-padding != true)
51 {
52 padding-right: $navbar-padding;
53 padding-left: $navbar-padding;
54 }
40 } 55 }
41 56
42 .navbar-branding img 57 .navbar-branding img
43 { 58 {
44 height: $navbar-branding-height; 59 height: $navbar-branding-height;
60 vertical-align: text-top;
45 } 61 }
46 62
47 /* .navbar .toggle-navbar-collapse 63 /* .navbar .toggle-navbar-collapse
48 ****************************************************************************/ 64 ****************************************************************************/
49 65
50 .toggle-navbar-collapse 66 .toggle-navbar-collapse
juliandoucette 2017/11/10 11:42:52 TOL: Are these properties that should apply to `.u
ire 2017/11/13 17:57:42 Done.
51 { 67 {
52 display: none; 68 display: none;
53 border: 0; 69 padding-top: ($navbar-branding-height / 2) + ($navbar-padding / 2);
54 background-color: transparent; 70 padding-bottom: ($navbar-branding-height / 2) + ($navbar-padding / 2);
55 cursor: pointer; 71
72 @if ($add-horizontal-navbar-padding != true)
73 {
74 padding-right: $navbar-padding;
75 padding-left: $navbar-padding;
76 }
56 } 77 }
57 78
58 .js .toggle-navbar-collapse 79 .js .toggle-navbar-collapse
59 { 80 {
60 display: block; 81 display: block;
61 82
62 @media (min-width: $tablet-breakpoint) 83 @media (min-width: $tablet-breakpoint)
63 { 84 {
64 display: none; 85 display: none;
65 } 86 }
66 } 87 }
67 88
68 .js .toggle-navbar-collapse .close-label
69 {
70 display: none;
71 }
72
73 .expanded .toggle-navbar-collapse .open-label
74 {
75 display: none;
76 }
77
78 .expanded .toggle-navbar-collapse .close-label
79 {
80 display: inline;
81 }
82
83 /* .navbar .navbar-collapse 89 /* .navbar .navbar-collapse
juliandoucette 2017/11/10 11:42:53 Suggest: Add padding/space to the <a> themselves i
juliandoucette 2017/11/10 11:42:53 Suggest: Add an active/hover/focus background colo
ire 2017/11/13 17:57:40 Do we want them to look more like buttons? I know
ire 2017/11/13 17:57:42 Same as previously answer. background-color applie
84 ****************************************************************************/ 90 ****************************************************************************/
85 91
86 .navbar-collapse 92 .navbar-collapse
87 { 93 {
88 clear: both; 94 clear: both;
89 padding-top: 0;
90 95
91 @media (min-width: $tablet-breakpoint) 96 @media (min-width: $tablet-breakpoint)
92 { 97 {
93 float: right; 98 float: right;
94 clear: none; 99 clear: none;
95 padding-top: $navbar-padding-v + ($navbar-padding-v / 2);
96 } 100 }
97 } 101 }
98 102
99 [dir="rtl"] .navbar-collapse 103 [dir="rtl"] .navbar-collapse
100 { 104 {
101 float: left; 105 float: left;
102 } 106 }
103 107
104 .js .navbar-collapse 108 .js .navbar-collapse
105 { 109 {
106 display: none; 110 display: none;
107 111
108 @media (min-width: $tablet-breakpoint) 112 @media (min-width: $tablet-breakpoint)
109 { 113 {
110 display: block; 114 display: block;
111 } 115 }
112 } 116 }
113 117
114 .expanded .navbar-collapse 118 .expanded .navbar-collapse
115 { 119 {
116 display: block; 120 display: block;
117 } 121 width: 100%;
118
119 /* .navbar .nav
juliandoucette 2017/11/10 11:42:52 NIT/Suggest: Make the whole line and space above/b
120 ****************************************************************************/
121
122 .navbar .nav li
123 {
124 margin-bottom: $navbar-padding-v;
125 122
126 @media (min-width: $tablet-breakpoint) 123 @media (min-width: $tablet-breakpoint)
127 { 124 {
128 display: inline-block; 125 width: auto;
129 margin: 0 0 0 $navbar-padding-v;
130 } 126 }
131 } 127 }
132 128
133 [dir="rtl"] .navbar .nav li 129 /* .navbar .navbar-nav
130 ****************************************************************************/
131
132 @if ($add-horizontal-navbar-padding)
133 {
134 .navbar .navbar-nav
135 {
136 padding-bottom: $navbar-padding / 2;
137
138 @media (min-width: $tablet-breakpoint)
139 {
140 margin-right: -($navbar-padding / 2);
141 margin-left: -($navbar-padding / 2);
142 padding-bottom: 0;
143 }
144 }
145 }
146
147 .navbar .navbar-nav li
134 { 148 {
135 @media (min-width: $tablet-breakpoint) 149 @media (min-width: $tablet-breakpoint)
136 { 150 {
137 display: inline-block; 151 display: inline-block;
138 margin: 0 $navbar-padding-v 0 0;
139 } 152 }
140 } 153 }
141 154
142 .navbar .nav li:last-child 155 .navbar .navbar-nav a
143 { 156 {
144 margin-bottom: 0; 157 display: block;
158 padding-top: $navbar-padding / 2;
159 padding-bottom: $navbar-padding / 2;
160
161 @if ($add-horizontal-navbar-padding != true)
162 {
163 padding-right: $navbar-padding;
164 padding-left: $navbar-padding;
165 }
166
167 @media (min-width: $tablet-breakpoint)
168 {
169 padding-top: ($navbar-branding-height / 2) + ($navbar-padding / 2);
170 padding-right: $navbar-padding / 2;
171 padding-bottom: ($navbar-branding-height / 2) + ($navbar-padding / 2);
172 padding-left: $navbar-padding / 2;
173 }
145 } 174 }
LEFTRIGHT

Powered by Google App Engine
This is Rietveld