Columns layout and visibility

[Rocky Road]

Follows bug #379084

Now that most (sigh) css rules are in css file, I'll try to help fixing them.

== Unused or misnamed container ids ==

Looking at page.tpl.php, I see that the main content is organised with
the following hierarchy:

    #container
        $breadcrumb
        #container3
            #container2
                #container1
                     #col1
                     #col2
                     #col3

so selectors with #container0, container-1, container-2, container-3 will be ignored.
I removed them in the patch below.

Also, #container3 and #container2 are just bare wrappers (no extra content) which
just add confusion in the stylesheet. I propose to remove all rules for them,
as well as the elements themselves.

== Misuse of "right" css property ==

See http://www.w3.org/TR/1998/REC-CSS2-19980512/visuren.html#propdef-right
"This property specifies how far a box's right content edge is offset to the left of the right edge of the box's containing block. "

Then in #container1, I read
  width: 100%;
  right: 75%;

This implies that left is now -75%, and it sadly means that the offsets of all inner elements have to balance this strange offset.

Recall that width percentage is calculated with respect to the width of the generated box's containing block. ( http://www.w3.org/TR/1998/REC-CSS2-19980512/visudet.html#the-width-property )

== overflow settings ==

Most rules in the layout section define "overflow=hidden".
This makes the errors related to above one, very critical !

http://www.w3.org/TR/1998/REC-CSS2-19980512/visufx.html#overflow-clipping

Moreover, this property's (as well as "position"'s) default value is "inherit", so there's no need to repeat it in inner elements.
http://www.w3.org/TR/1998/REC-CSS2-19980512/cascade.html#value-def-inherit

Also, I'm not sure that defining overflow where width=100% makes sense.

== Columns positionning ==

Why not leaving "float:left" doing the job of setting the "left" value ? just set "width"
I'd tend to see float and left as conflicting ways of specifing horizontal position of the box.
Better choose one and stick to it.
As I prefer the more modern fluid layouts, I'd try float.

== The patch ==
Apply patch for bug #379084 first !

Values are only rough settings, there is some work to get a good rendering, I leave this to you.
But I'm sure it should be far easier with this new structure.

[Ddorda (ddorda)]

on #container add: "padding-left: 17;

[Michael Lustfield (michaellustfield)]

I hope this isn't too much to ask... but... Would you be willing to apply all the patches you've created in your own branch and propose another merge. I'd like to be sure I obtain the same result you are looking at. I'd really appreciate either the single merge proposal or a single patch file. Because I committed one of your patches to the main branch our code has diverged and the merge may be the easiest approach.

I have a feeling that the code you did will be significantly better and I have a hunch I will be forced to accept your merge. You should be able to just request another review of the currently standing proposal.

[Rocky Road (m-baert)]

I checked again just in case I had messed thes patches.

The branch lp:~m-baert/ubuntu-drupal-theme/theme-patches was created
on your request specifically to apply the patches and tests the
results, which are still displayed on
http://sandbox.rocky-shore.net/drupal/ (login as "UD-tester", password
"loco", to enable the theme).

The revision 96 is interesting from a testing and versioning point of
view because it contains all major code structural changes, while NOT
changing the visual result. Please report to me it it was the case.

Here are the commands you should be able to reproduce:

----
pushd /tmp

# from bug #379084 : Replace inline css with suitable classes
wget http://launchpadlibrarian.net/27023257/replace-inline-css.patch

# from bug #379205: Columns layout and visibility
wget http://launchpadlibrarian.net/27029798/columns-layout.patch
wget http://launchpadlibrarian.net/27029342/columns-css.php

# Now checkout the branch, and check the patches
bzr branch lp:~m-baert/ubuntu-drupal-theme/theme-patches

bzr diff -r95..96 theme-patches > r95-96.patch
diff replace-inline-css.patch r95-96.patch

bzr diff -r96..97 theme-patches > r96-97.patch
diff columns-layout.patch r96-97.patch

# In both cases, only source files timestamps differ

----

The merge request you received should contain a patch file equivalent
to that obtained with:

bzr diff -r95..98 theme-patches > r95-98.patch

Using it directly would IMHO just help you NOT understanding the
changes.

All I see I could have done better is pushing an intermediate revision
between 96 and 97, where only illegal or orphans rules would be
removed, so that no visual difference should appear.

But is it worth ?

[Michael Lustfield (michaellustfield)]

I attempted to use your branch. I'm using it currently on my staging website (http://staging.profarius.com/).

Please look at the changes in your branch cause. Issues also occur in the /admin section.

[Michael Lustfield (michaellustfield)]

This bug was improperly marked as fix committed. The patches was not committed to the source nor were they tested enough to be committed.

However, I have pushed a fix to this issue in r97.