Support from Acquia helps fund testing for Drupal Acquia logo

Comments

rjgoldsborough’s picture

Here's a patch to start things off. A less alerting color but still gets the point across I think. Also, I thought the header region looked better and made more sense to be full width. After all, the header is more than just the little block it shows now. I also thought the descriptions looked better centered. Also looks good across other color schemes.

threewestwinds’s picture

Status: Needs review » Reviewed & tested by the community

Tested in Firefox 3.6, Chrome 7, and IE 8 against a fresh install of 7.x-dev. Makes the page look very much nicer, and the header block is more accurate about where blocks actually end up on the page. The changes are minor enough that I didn't feel the need to fire up more browsers than that.

The grey is somewhat hard to see against a white background, but still infinitely less eye hurting than the yellow it replaces.

Style in the new CSS meets standards.

Jeff Burnz’s picture

Should we make all the region names the same font, font size etc? The featured one looks the oddest having the text shadow.

threewestwinds’s picture

Status: Reviewed & tested by the community » Needs work

Agreed that the shadow looks a bit weird and the size should be standardized.

To be honest though, I'm not sure the proper way to set font-size in a situation like this. Using em or % inherits the size changes from the parent element (meaning either Featured doesn't shrink or the rest of the blocks do), but using an absolute size is bad for accessibility (since the size doesn't change properly in all browsers).

We could do a "#featured .page-admin-structure-block-demo .block-region" specific fix, but that feels like a hack to me, and would need to be fixed every time someone modified the #featured font size. Yuck.

I'll roll a patch if anyone can help me with how to make that change.

Jeff Burnz’s picture

Use a keyword, i.e.

.page-admin-structure-block-demo .block-region {font-size: medium;}

A keyword font-size will lock the relative size to the browser default and scale with normal text re-sizing.

threewestwinds’s picture

Status: Needs work » Needs review
FileSize
504 bytes

My first patch ever. Thanks for the tip on keyword font-size - you learn something new everyday.

Same as the last patch, plus two lines, one to remove the header text-shadow and another to set all the block-demos to a uniform "medium" text size.

dcrocks’s picture

FileSize
97.41 KB
53.67 KB

I don't know if this is the right place to put this but I just ran into a weird problem. On a vanilla 7.x-dev system dated 11/14, if bartik is not the default theme, when using admin/structure/blocks, the line " Demonstrate block regions(*theme name*) " never shows bartik, even though bartik is the theme whose block structure is currently displayed. See attached, where garland was the current theme but I was displaying bartik's blocks. The block structure of the current theme(garland) is displayed. If bartik is default, it works fine. If you choose another theme that is not default, it works fine. It only seems to have a problem with the 1st theme in the list, in my case bartik.

ps. Can the " Exit Block Region Demonstration " message be on a transparent background. If you have a region that displays top left, you can't see the label.

threewestwinds’s picture

Category: bug » feature

A quick search on my part doesn't show anything about your problem - if you open a new issue, I'll try and take a look at it.

But not here - this is just a quick cosmetic fix. This shouldn't even be a bug report, I suspect, now that I think about it. I'll go with the handbook: "Feature Requests are for situations where the software works as designed, but the design can be improved. " (http://drupal.org/node/73179)

dcrocks’s picture

If I understand you correctly I need to create 2 new issues: a bug report about the "demonstrate regions" command, and a feature request about adding transparency to the "exit demonstrate regions" message.

threewestwinds’s picture

I did a little bit of looking at the block bug you mentioned - it's a Garland specific problem. At least, I think so - Analytic displays the region demonstration properly (http://drupal.org/project/analytic just happened to be the first theme I grabbed that defined additional regions).

Yes, two issues - one on the "block.module" component for the transparency request (that's where the CSS for the tab you'd like transparent resides), and one on the "Garland Theme" component for the improper regions demonstration.

I know they both sound like they belong here, but they're actually separate issues from different modules. ;)

dcrocks’s picture

Ok, I'll do the transparency issue, but I don't think the other is a garland problem. I tried other themes as well. If I can put it more generally, you can not use the the "Demonstrate Block Regions" command on any theme that appears first in the list of themes in the admin/structure/blocks overlay page if it is not the current default theme. Instead the regions for the default theme will be displayed. It appears that the 1st theme in the list is always the theme that was default when you logged in.

ps. After further checking, after caches are cleared, the current default theme is the 1st in the list and there is no problem. But I don't think you can expect people to clear cache every time they change the default theme.

jensimmons’s picture

Category: feature » bug

Ok, everything in comment #7, 8, 9, 10, and 11 should not be on this issue. Let's keep talking about the original issue and comments #1–6 here. Please do not say anything else regarding comments #7–11. Open new issue(s) for that.

jensimmons’s picture

FileSize
811 bytes

Before:

After:

tim.plunkett’s picture

Status: Needs review » Reviewed & tested by the community

Code looks great, and the regions look even better.

threewestwinds’s picture

Status: Reviewed & tested by the community » Needs work
FileSize
33.82 KB

When I apply it, the header region stays narrow, and I don't see any lines in your patch that would have made it expand like your images.

I definitely prefer the yellow you reintroduced. It makes the blocks immediately visible, regardless of background. Am I missing something?

tim.plunkett’s picture

Status: Needs work » Reviewed & tested by the community

I'm not sure how the header expanded for that screenshot, because it won't and shouldn't. After #845834: Fix Bartiks Header because its totally borked, that's where header is, and where blocks put in it will go. Any tweaking to get the header to stretch just for this page is too much.

So the screenshot is wrong, but the patch is right.

tim.plunkett’s picture

Assigned: Unassigned » tim.plunkett
Status: Reviewed & tested by the community » Needs review

On second thought, this is worth a quick conversation with Jen. Will probably be right back to RTBC.

jensimmons’s picture

Hm, first an explanation about the latest patch. I'm here at BADCamp with webchick and timplunkett and we had a little chat about the design of these markers. Webchick was pushing hard for us to use a yellow very similar to what's there before this patch, since we are about to push D7 out the door, and it's too late for things the change the look to much. And I don't mind the yellow too much — the tan is prettier, but it blends into the theme too much. These markers need to jump off the page and let people know — hey this is not part of the theme, these are thingys marking where the regions are. So I came up with a new design. This new code provides markers that have:

  • Consistent fonts, using the same font that is in the Seven theme.
  • Mostly consistent font sizes — specifically the featured region is no longer huge. Most are the same, except for the footer regions (which are still a bit smaller).
  • Adds a dotted line around everything, so that the markers look like they jump out a bit more / float above the page. This also makes them more accessible — they will show up on a yellow background, and are more visible to someone who doesn't see color in the same ways other people do.

And congrats to threewestwinds!!! Your first patch :D YEAH!!!!!!!!!!!! And yeah rjgoldsborough for working on this too.

jensimmons’s picture

FileSize
905 bytes

Re comment #15 — Great point. Here's new patch with a wider header marker.

When reviewing this patch, please put blocks in all the regions, and test to make sure this CSS doesn't affect the content on the page.

threewestwinds’s picture

Status: Needs review » Reviewed & tested by the community

Tested blocks in all regions, no difference in appearance between patched and unpatched version except on the page in question.

Looks good to me.

tim.plunkett’s picture

I second this.

jensimmons’s picture

Screenshot of the last patch for webchick. Meow.

webchick’s picture

Status: Reviewed & tested by the community » Fixed

Committed #19 to HEAD. Thanks!

Status: Fixed » Closed (fixed)

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