Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.The screenshots should make it pretty clear. One-liner.
| Comment | File | Size | Author |
|---|---|---|---|
| #20 | vertical-tabs-summaries-stark_03.patch | 2.62 KB | tstoeckler |
| #16 | vertical-tabs-summaries-stark_02.patch | 2.41 KB | tstoeckler |
| #16 | stark-summaries.png | 28.56 KB | tstoeckler |
| #3 | patch.png | 24.22 KB | tstoeckler |
| #3 | patch-smaller.png | 23.7 KB | tstoeckler |
Comments
Comment #1
Dries CreditAttribution: Dries commentedInstead of making the "second row" smaller, it might be better to remove the link from the "second row"?
Comment #2
mcrittenden CreditAttribution: mcrittenden commentedI agree with Dries. If you make the test smaller, you still (perhaps even more so) face the problem of people thinking that the description text is a different link than the heading text for each one. Better to remove the link altogether. We could still make the text smaller in addition to that, though.
Comment #3
tstoecklerI like that idea. Removing the html a-tag, though, you lose that pretty "the whole tab is clickable" thing (try it, the only pixel that isn't clickable is the border between two tabs, isn't it sweet...). So i simply changed the font color for those particular links to black. (Related note: Is there any "code style"-thing for the order of css attributes?) See screenshot.
Even though I read Dries' "Instead", I also attached a screenshot that has the above and the smaller font-size for comparison. I personally think with the different color that is not necessary, though.
I wondered, though about the "strong" tag of the tab-title, that was deemed so important in the initital VT-patch that it was added back in after someone removed it, for "semantic reasons". So i wondered, why these semantics (which clearly make sense) aren't to be seen by the end user, and I saw that the bold-tag, that strong otherwise has, is specifically removed for these VT-tab-titles. So I added that back in and attached a screenshot how that looks together with the attached patch. I like it best, actually, although you lose the distinction of the active tab, which is always bold.
Comment #4
mcrittenden CreditAttribution: mcrittenden commentedI agree that the third version looks the best, but I think that sort of goes against the whole point of that theme, which seeks simplicity wherever possible. I think we should be aiming for usability over pretty-ness in a theme like this.
With that in mind, I'd vote for the second version, with the text colored differently and a little smaller. Yeah, it adds some styles, but I think the usability improvement is a big one. Contrast this with the third version, which isn't a usability improvement over the second version (in fact, it might be seen as less usable as the active tab doesn't stand out as much as you pointed out), and just looks a little better.
Comment #5
mrfelton CreditAttribution: mrfelton commentedMy vote goes for version 2 also. A massive visual improvement and +1 for usability - the page is much clearer.
Comment #6
tstoeckler@mcrittenden: Actually the third version is the only one which actually removes one line of code, namely the "font-weight: normal" in the ".vertical-tabs-list li strong" element which overrides the normal "stong {font-weight: bold;}" code. Therefore your (first) argument should be for the third version.
Concerning usability: I personally think things such as this should be handled on the theme layer (therefore added in Garland's code, not core's default output, but let's hear what Dries says...
Comment #7
mcrittenden CreditAttribution: mcrittenden commentedtstoeckler: Good point. Since stark is the only affected theme, why not fix it directly in stark rather than in core css?
Comment #8
tstoecklerBecause Stark is meant to reveal core's CSS. It is not a theme in its usual sense. The only CSS that Stark has, is a bit of layout, not any real theming.
I do believe that core should output some 'sensible defaults' when it comes to markup, even though you can do anything with a custom theme nonetheless. Just as it makes sense to color links in blue by default, even though any theme can do this its way.
Comment #9
webchickTagging.
Comment #10
mcrittenden CreditAttribution: mcrittenden commentedI say let's roll a patch with solution #3 (make all the tab headings bold) to review.
Comment #11
kkaefer CreditAttribution: kkaefer commentedWe could move the font-weight: normal to Garland and make it black. Additionally, we could use
<small>instead of span for the descriptions.Comment #12
Bevan CreditAttribution: Bevan commentedDon't make all of them bold. Attention does not need to be drawn to titles – users seek them out on their own. Perhaps just the first one I think is fine. http://drupal.org/files/issues/patch-smaller.png is great. Possibly also indent the summary a 0.5em.
Comment #13
tstoeckler@Bevan: Note that this is not about theming. We're not talking about how Garland displays the stuff but Stark (read: Drupal core without any theme). Therefore I am +1 for #11, which would keep only the selected one bold (but in Garland's code, not Drupal's). The indent of the summary is arguable but definitely belongs to Garland's territory (again: not Drupal core's output).
Comment #14
Bevan CreditAttribution: Bevan commentedIs Stark's goal to provide minimalistic CSS? Or just HTML? Does it not want to provide – at the very least – good – albeit minimalistic – defaults?
Comment #15
tstoecklerExactly.
0.5em indent isn't minimalistic, though.
And it is debatable (as seen above) whether first tagging something "strong" and then font-sizing it normal is minimalistic. I'd say no again.
Comment #16
tstoecklerAttached a new patch. Followed kkaefer's suggestion and turned < span > into < small >. Garland still looks the same and it allowed to delete a few lines from vertical-tabs.css, which is good, IMO.
Attached a screenshot for how it looks it Stark now. From my POV this should be good to go.
Don't know why this was at "needs review", but now it actually does.
Comment #18
tstoecklerComment #20
tstoecklerThe recent vertical-tabs CSS clean-up issue broke this.
Rerolled and tested locally: still works as expected. Screenshots, are still the same (#16).
Comment #21
webchickThis looks good to me. I've committed this to HEAD. We can always make touch-ups later if we need to, but this is a big improvement. Thanks, tstoeckler! :)