The 20px top margin on the body should probably not be hard-coded into the CSS file.

If a site has a design which expects the foreground items to sit correctly on the body background image, shifting the content down 20px can break things (see attached jpg).

I think there should be a settings page with an option to turn off the shifting of page content. It would create a javascript setting variable that could be read by the .js file to change the body top-margin based on the setting.

I can see it in my mind. Now all I need to do is write it!

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

sun’s picture

I also had issues with that. With the new jQuery code in 5.x-2.2 I thought of adding a class to the BODY to allow theme developers to react on this margin - or on the existence of admin_menu at all.

Something like

$('body').addClass('admin_menu');

and if needed in a theme:

.foo { position: absolute; top: 100px; }
.admin_menu .foo { top: 120px; }

How about that?

jjeff’s picture

Yeah, I like that approach too... But I'll bet a lot of people would like it to "just work" with their theme without having to go in and adjust CSS. It seems like a simple setting for this would solve the problem.

I'd vote for both a setting AND a class on the body.

sun’s picture

Hm... Since admin_menu does not have any settings yet, and we really tried hard to find solutions that do not require module settings, we need a thorough plan about what should be configurable, and how. For example, this proposed setting would not be a module setting, but rather a theme setting.

We are talking about customizing the output of admin_menu, so you might read http://drupal.org/node/109295 (especially my latest follow-up) for further information on this topic.

sun’s picture

Status: Active » Needs review
FileSize
1.67 KB

First take on the body class.

jjeff’s picture

FileSize
4.07 KB

Wow! I can't keep up! :-)

Here's a patch I've rolled to add a settings page (and a help page) to switch on/off the 20px margin at the top of the page.

Why have you avoided a settings page? I understand the argument of not wanting to add yet another page to Drupal's admin section. However, I would think that a settings page will be the first thing that a site administrator will look for. By going to great lengths to move the settings onto other pages (such as the form_alter() for the Devel settings page), I fear that administrators will just assume that the module has no customization settings, and never find them scattered across the various settings pages.

That being said, you're correct that this setting should probably be per theme. The frustrating part is that this ups the complexity (and downs the usability) exponentially.

As for the editing of CSS, I guess it could be argued that if the admin is having these problems, they've probably (like myself) got a complex custom theme on their site and editing CSS might be within their grasp. But in general, I'm always dubious of any module that asks you to write any custom code in order to make it work.

...just my 0.02 Euro

sun’s picture

Status: Needs review » Fixed

Thanks! I've committed a slightly modified version of your patch now.

Anonymous’s picture

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for two weeks with no activity.