UD Theme

Merge lp:~m-baert/ubuntu-drupal-theme/theme-patches into lp:~ubuntu-drupal-devs/ubuntu-drupal-theme/6.x

  1. theme-patches
  2. Merge into 6.x
Proposed by Rocky Road
Status: Rejected
Rejected by: Michael Lustfield
Proposed branch: lp:~m-baert/ubuntu-drupal-theme/theme-patches
Merge into: lp:~ubuntu-drupal-devs/ubuntu-drupal-theme/6.x
Diff against target: None lines
To merge this branch: bzr merge lp:~m-baert/ubuntu-drupal-theme/theme-patches
Reviewer Review Type Date Requested Status
Michael Lustfield (community) Disapprove
Review via email: mp+6735@code.launchpad.net
To post a comment you must log in.
Revision history for this message
Rocky Road (m-baert) wrote :

Simplified containers structures and added css flexibility for either 1, 2, or 3 columns layouts.
Property values adjusted for 3 columns only.

Needs to be tested and likely adjusted for other layouts, administration pages, views etc...
No new properties, no tricks, there should not be any cross-browsers issue, unless browser-specific styling was already faulty (I didn't check that).

Revision history for this message
Michael Lustfield (michaellustfield) wrote :

I won't yet disapprove of the merge, however I need to see some fixes.

For this patch to be accepted, it needs to be at least nearly as stable as the current code.

When I applied these changes:
1. It felt like a 3-column layout was being forced
2. The center column wouldn't expand to needed width
3. When on the admin page, the center column was set below the left column content

Approved changes:
1. /admin/build/block appears to function better

Overall - The changes made break the theme. I like the improvements I'm seeing, but it is currently too unstable to merge into a stable branch.

Side Note: Please remember that this page can have a variable width to it as well. Some of the changes seem to neglect that.

review: Needs Fixing
Revision history for this message
Rocky Road (m-baert) wrote :

I'm not trying to sell you a Mac Burger,

Your theme was previously hiding at least right column when both where enabled. I find out why, (an overall mess with css rules) fixed the cause, spent hours to explain why and how,
for theme future maintenance's sake.

I made it clear enough that I don't want to fix the details. Fixed the logical structure.

Your answer just makes me think it was not worth.
Do what you want.

Revision history for this message
Michael Lustfield (michaellustfield) wrote :

These patches have proved to break the theme significantly. Rather than accept these changes, I have restructured the code myself. These changes have been pushed to main and fix what this was meant to.

review: Disapprove

Unmerged revisions

98. By Rocky Road

Adjusted layout properties for 3 columns pages.
(Ddorda + RockyRoad)

97. By Rocky Road

Simplified layout of containers and columns.

96. By Rocky Road

Replaced html head generated css with suitable body classes. Should NOT cause any change in visual result.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'page.tpl.php'
2--- page.tpl.php 2009-04-02 19:14:25 +0000
3+++ page.tpl.php 2009-05-21 23:45:14 +0000
4@@ -7,12 +7,11 @@
5 <title><?php print $head_title ?></title>
6 <?php print $head; ?>
7 <?php print $styles; ?>
8- <?php print $csheet; ?>
9 <?php print $scripts; ?>
10 <!--[if gte IE 7]><?php print ie_styles(); ?><![endif]-->
11 <!--[if lte IE 6]><?php print ie6_styles(); ?><![endif]-->
12 </head>
13- <body>
14+ <body<?php print phptemplate_body_class($left, $right); ?>>
15 <!-- IE Warning -->
16 <!--[if gte IE 5]><?php print $iebanner; ?><![endif]-->
17 <!-- Layout -->
18@@ -49,12 +48,8 @@
19 <!-- /#header -->
20 <div id="container">
21 <?php echo $breadcrumb; ?>
22- <div id="container3">
23- <div id="container2">
24- <div id="container1">
25- <?php print $columnarea; ?>
26- </div>
27- </div>
28+ <div id="container1">
29+ <?php print $columnarea; ?>
30 </div>
31 </div>
32 <!-- /#container -->
33
34=== modified file 'style.css'
35--- style.css 2009-04-17 02:07:43 +0000
36+++ style.css 2009-05-22 01:29:15 +0000
37@@ -161,7 +161,7 @@
38 }
39
40 #container {
41- padding-top: 10px;
42+ padding-left: 17px;
43 }
44
45 /*
46@@ -219,7 +219,7 @@
47 .breadcrumb {
48 color: #cccccc;
49 font-size: 1em;
50- margin-top: -9px;
51+ margin-top: 4px;
52 left: 26%;
53 padding: 0;
54 position: absolute;
55@@ -1159,56 +1159,80 @@
56 padding: none;
57 }
58
59+
60+/* outer container, after #header */
61 #container {
62 position: relative;
63 background-color: #ffffff;
64 z-index: 2;
65-}
66-
67-#container0{
68- float: left;
69- width: 100%;
70- position: relative;
71- right: 75%;
72-}
73-
74-#container1{
75- float: left;
76- width: 100%;
77- position: relative;
78- right: 75%;
79-}
80-
81-#container2 {
82- float: left;
83- width: 100%;
84- position: relative;
85+/* FIXME effective box height is 30px here */
86+ padding:0;
87+}
88+
89+/* inner container */
90+#container1 {
91+ width: 100%;
92+ padding: 15px 10px;
93+/* FIXME effective box height is 0 here */
94+}
95+
96+/* let inner elements stack horizontally */
97+#container1 div {
98+ float: left;
99+/* FIXME enabling this hides rounded corners
100 overflow: hidden;
101+*/
102+}
103+
104+/* reset inner elements float to default @testme */
105+#container1 div * {
106+ float:none;
107 }
108
109 #col1 {
110- float: left;
111 width: 23%;
112- position: relative;
113- left: 76%;
114- overflow: hidden;
115+ left: 0;
116 }
117
118 #col2 {
119- float: left;
120- width: 73%;
121- position: relative;
122- left: 78%;
123- overflow: hidden;
124+ width: auto; /* this should occupy all free space ... @testme */
125+ margin: 0 15px;
126 }
127
128 #col3 {
129- float: left;
130- width: 73%;
131- position: relative;
132- left: 80%;
133- overflow: hidden;
134-}
135+ width: 23%;
136+ right: 0;
137+}
138+
139+/* sidebar left: ''; sidebar right: '' */
140+body.with-no-sidebar #container1 {
141+ width:90%; /* sample property, for demo */
142+}
143+body.with-no-sidebar #col2 {
144+ width:100%; /* sample property */
145+}
146+/* sidebar left: 'yes'; sidebar right: '' */
147+}
148+body.with-sidebar-left #col1 {
149+ width:25%;; /* sample property, for demo */
150+}
151+
152+/* sidebar left: 'yes'; sidebar right: 'yes' */
153+body.with-both-sidebars #container1 {
154+}
155+body.with-both-sidebars #col1,
156+body.with-both-sidebars #col3 {
157+ width:18%;
158+}
159+body.with-both-sidebars #col2 {
160+ width:58%;
161+}
162+
163+/* sidebar left: ''; sidebar right: 'yes' */
164+body.with-sidebar-right #col3 {
165+ width:25%; ; /* sample property, for demo */
166+}
167+/* end of layout */
168
169 #content {
170 position: relative;
171
172=== modified file 'template.php'
173--- template.php 2009-04-17 02:07:43 +0000
174+++ template.php 2009-05-21 23:42:11 +0000
175@@ -74,39 +74,33 @@
176 * 2. PHPTemplate Functions
177 */
178
179-function phptemplate_body_class($left, $right, $header = '', $footer = '', $var_show_blocks) {
180- if ($var_show_blocks) {
181- if ($left != '' && $right != '') {
182- $class = 'sidebars';
183- }
184- else {
185- if ($right == '' && $left == '' && ($header != '' || $footer != '')) {
186- $class = 'sidebars';
187- }
188- if ($left != '') {
189- if ($header != '' || $footer != '') {
190- $class = 'sidebar-mix';
191- }
192- else {
193- $class = 'sidebar-left';
194- }
195- }
196- if ($right != '') {
197- if ($header != '' || $footer != '') {
198- $class = 'sidebar-mix';
199- }
200- else {
201- $class = 'sidebar-right';
202- }
203- }
204- if ($right == '' && $left == '' && $header == '' && $footer != '') {
205- $class = 'sidebar-footer';
206- }
207- }
208- }
209+/**
210+ * Builds a class attribute reflecting the presence of sidebars.
211+ * The result is supposed to be inserted in the <body> start tag
212+ *
213+ * @return a string of the form " class=\"$class\"" , where $class is one of:
214+ * - 'with-both-sidebars'
215+ * - 'with-sidebar-left'
216+ * - 'with-sidebar-right'
217+ * - 'with-no-sidebar'
218+ */
219+function phptemplate_body_class($left, $right) {
220+ /* stronger code: take care of various ways of meaning 'empty' */
221+ $lshow = (isset($left) && !empty($left)) ? 1 : 0;
222+ $rshow = (isset($right) && !empty($right)) ? 1 : 0;
223
224- if (isset($class)) {
225- return ' class="'. $class .'"';
226+ if ($lshow) {
227+ if ($rshow) {
228+ return ' class="with-both-sidebars"';
229+ } else {
230+ return ' class="with-sidebar-left"';
231+ }
232+ } else {
233+ if ($rshow) {
234+ return ' class="with-sidebar-right"';
235+ } else {
236+ return ' class="with-no-sidebar"';
237+ }
238 }
239 }
240
241@@ -320,7 +314,6 @@
242 }
243
244 function phptemplate_preprocess_page(&$vars) {
245- $vars['csheet'] = ubuntudrupal_columnsheets($vars['left'], $vars['right']);
246 $vars['iebanner'] = ubuntudrupal_iebanner();
247 $vars['countdown'] = ubuntudrupal_countdown();
248 $vars['topmenu'] = ubuntudrupal_whichmenu($vars['primary_links']);
249@@ -344,59 +337,6 @@
250 * 3a. UbuntuDrupal Variable Overrides
251 */
252
253-function ubuntudrupal_columnsheets($left, $right) {
254- $center = 0;
255- $lshow = $left ? 1 : 0;
256- $rshow = $right ? 1 : 0;
257-
258- if ($lshow && $rshow) {
259- $column = 3;
260- $firstcolumn = 20;
261- $secondcolumn = 60;
262- $center = 60;
263- $lastcolumn = 20;
264- $padding = 1;
265- }
266- elseif (!$lshow && $rshow) {
267- $column = 2;
268- $firstcolumn = 75;
269- $secondcolumn = 25;
270- $lastcolumn = 25;
271- $padding = 1;
272- }
273- elseif ($lshow && !$rshow) {
274- $column = 2;
275- $firstcolumn = 25;
276- $secondcolumn = 75;
277- $lastcolumn = 75;
278- $padding = 1;
279- }
280-
281- $colsheet = '<style type="text/css">' . "\n";
282-
283- $colsheet .= '#container' . $column . ' {' . "\n";
284- $colsheet .= 'float:left; width:100%; position:relative; overflow:hidden; }' . "\n";
285-
286- $colsheet .= '#container' . ($column-1) . '{' . "\n";
287- $colsheet .= 'float:left; width:100%; position:relative; right:' . $lastcolumn . '%; }' . "\n";
288-
289- $colsheet .= '#container' . ($column-2) . '{' . "\n";
290- $colsheet .= 'float:left; width:100%; position:relative; right:' . $secondcolumn . '%; }' . "\n";
291-
292- $colsheet .= '#col1 {' . "\n";
293- $colsheet .= 'float:left; width:' . ($firstcolumn-($padding*2)) . '%; position:relative; left:' . ($lastcolumn+$center+$padding) . '%; overflow:hidden; }' . "\n";
294-
295- $colsheet .= '#col2 {' . "\n";
296- $colsheet .= 'float:left; width:' . ($secondcolumn-($padding*2)) . '%; position:relative; left:' . ($lastcolumn+$center+($padding*3)) . '%; overflow:hidden; }' . "\n";
297-
298- $colsheet .= '#col3 {' . "\n";
299- $colsheet .= 'float:left; width:' . ($lastcolumn-($padding*2)) . '%; position:relative; left:' . ($lastcolumn+$center+($padding*5)) . '%; overflow:hidden; }' . "\n";
300-
301- $colsheet .= '</style>' . "\n";
302-
303- return $colsheet;
304-}
305-
306 function ubuntudrupal_iebanner() {
307 if (theme_get_setting('banner_display')==1) {
308 $bannertext = theme_get_setting('banner_text');

Subscribers

People subscribed via source and target branches

to all changes:
to status/vote changes: