There is an IMHO ugly 2px-margin below inactive tabs on overlays.

The attached patch removes this margin so the tabs sit snugly on top of he content area.

An inactive tabs being greyed out is enough of a discriminator to distinguish it clearly from an active tab (white) which bleeds nicely into the white background of the content area.

Tested to work on Windows XP with IE6, IE7, IE8, Firefox 3.5, Chrome and on Mac with Firefox 3.5, Safari 4, Chrome.

Screenshot (after patch) from FF3.5 on Mac

Searched the issue queue on this but found nothing.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

aspilicious’s picture

I like this improvement.

karschsp’s picture

Status: Needs review » Reviewed & tested by the community

I agree, I think this looks good. Marking RTBC.

Dries’s picture

Status: Reviewed & tested by the community » Fixed

Committed to CVS HEAD.

Status: Fixed » Closed (fixed)

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

seutje’s picture

Title: Ugly margin below inactive tabs on overlay » make overlay tabs look like the mockups
Status: Closed (fixed) » Needs review
FileSize
1.15 KB

was this not by design?

attached patch should also make IE more happy, as it doesn't rely on inline-block displau

reglogge’s picture

To be honest, I didn't lok at the mockups beforehand, I just always had the impression that this little margin between already greyed out tabs and the white background of the overlay looked more like a css-bug than a conscious design decision.
I agree however with switching from display: inline to float: left for the tabs.
The attached patch incorporates this change from #5 but keeps the snug fit of the tabs to the overlay.
Take your pick.

reglogge’s picture

forgot to attach a screenshot

Kiphaas7’s picture

Err, I think the switch from float to inline was made in some issue earlier... Exactly to avoid IE problems? Besides, IE support display inline-block, as long as the element is a "natural" inline element.

Dries’s picture

I agree with reglogge in #7. I also thought that the original 'gap' was a CSS issue. In other words, I think this patch would be a regression.

Bojhan’s picture

reglogge’s picture

@Bojhan: This image is far too small to see anything in it and can't be enlarged at Flickr. Any other links where one could look at the original design?

Bojhan’s picture

@reglogge I know, its annoying - but you need to be logged into flickr to see it in full size

reglogge’s picture

@bojhan: now I can see the original spec - it has the 1px margin between the deactivated tab and the overlay body.

I still think that the margin looks odd though - more like a bug than a feature - and Dries agrees in #9.

I'd much rather leave it as it is now.

Bojhan’s picture

Well Dries can be wrong, and in this case he is. I dont think it looks bad, I think it looks good - its not a bug as it allows for visual clarity in what you are currently in, and what is not active.

aspilicious’s picture

Status: Needs review » Fixed

I think we all agree (except for Bohjan) that the gap looks weird, no one else complained...
Closing this!

Bojhan’s picture

Status: Fixed » Needs review

lol, no offense - but mark boulton designed it this way.

aspilicious’s picture

So... it looks rly rly rly strange....
I showed it to my prof HCI and she said the following:

"I can see what he is trying to do with the margin, but I understand that most people get confused by it."

If I look at those mockups a lot has changed, I don't see why we can't change this?
Is mark a God? No he isn't. Am I God, no I'm not. I only collect opinions from core people and people in my environment that are going to use drupal in the future and besides of you (and probably Mark) nobody has said: "hey lets keep the margin! It is better cause of ... " in he last 2 months.

In my opinion the colour difference does it all and I have the feeling adding the margin is just overkill...
But that's my opinion, I'll leave this open for discussion Bohjan but if nobody replies to this before 15 april I'll close this again.

yoroy’s picture

Status: Needs review » Needs work

First of, I don't think any variant of this should be considered 'confusing'. (Nor are we discussing who's god here.)

Both approaches work.

It's a matter of how much emphasis you put in the styling of active versus inactive tabs. We are not our own target audience here, not even (especially! :) Dries. We're designing for people who won't notice.

_With_ the gap is better, especially with multiple inactive tabs. It adds clarity:

Untitled-1 @ 100% (Layer 2, RGB)

view image: http://img.skitch.com/20100323-bw7itkqj5trc9w943uxns2pf22.png

kika’s picture

subscribing

james.elliott’s picture

subscribing

casey’s picture

Status: Needs work » Needs review
FileSize
543 bytes

I actually agree using the gap for inactive tabs looks better.

aspilicious’s picture

Well...

If you all agree, this is fine for me. But I'm NOT responsible for every issue being opened cause of this ;)

yoroy’s picture

#21: overlay-tabsspacing.patch queued for re-testing.

Bojhan’s picture

Status: Needs review » Reviewed & tested by the community

Off to RTBC, about time we fix this issue.

If any follow ups are caused by this issue I think its reasonable we hold aspilicious responsible.

yoroy’s picture

Appearance | d7

looking great and it's really comforting to know we can blame aspilicious for everything afterwards :)

ksenzee’s picture

+1 to implementing the mockups as designed. Also +1 to getting this fixed and out of the queue. And if aspilicious doesn't want to take the blame I will. ;)

Dries’s picture

Status: Reviewed & tested by the community » Fixed

Committed to CVS HEAD. Thanks.

Status: Fixed » Closed (fixed)

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

Bojhan’s picture

Status: Closed (fixed) » Active

Seems like this wasn't actually committed? As per todays checkout I do not see the 1px for inactive tabs.

Bojhan’s picture

http://drupal.org/cvs?commit=398852 Guess we should blame aspilicious for this, just for the sake of it

aspilicious’s picture

Status: Active » Needs review
Issue tags: -Needs design review, -overlay +quickfix
FileSize
466 bytes

Nothing wrong with the commit.
We have to blame seven theme.

#25 is how it looks with the patch

Hopefully quickfix

ksenzee’s picture

Status: Needs review » Reviewed & tested by the community

That is specific and limited enough that even I can RTBC it safely. :)

yoroy’s picture

I've been missing the 1px below inactive tabs too lately. Would love to see them re-introduced.

Dries’s picture

Status: Reviewed & tested by the community » Fixed

Committed to CVS. For real.

Status: Fixed » Closed (fixed)
Issue tags: -quickfix

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