Implement Seven style guide for core overlay

There's some responsiveness and breakpoint integration we can port from #1829326: Convert Overlay to leverage core breakpoints and media queries to determine its presence and styling.

+++ b/core/modules/overlay/overlay-child.cssundefined
@@ -36,12 +39,17 @@
 #overlay-title-wrapper {
   overflow: hidden;
+  padding: 15px;
 }
 #overlay-title {
   color: #fff;
@@ -64,75 +72,18 @@

@@ -64,75 +72,18 @@
 
 #overlay-close-wrapper {
   position: absolute;
-  right: 0; /* LTR */
+  right: 46px; /* LTR */
+  top: 18px;

As much as possible let's use ems for these types of placement and padding values.

+++ b/core/modules/overlay/overlay-tabs.cssundefined
@@ -0,0 +1,83 @@
+.secondary-tabs__item__link {
+  border-bottom: 1px solid transparent;
+  color: #0074bd;
+  display: block;
+  margin-left: 15px;
+  padding: 15px 0 5px 0;
+  text-decoration: none;
+  transition: all 0.3s;

We'll need to pull base styles apart from theme styles.

+++ b/core/modules/overlay/templates/overlay.tpl.phpundefined
@@ -24,13 +24,13 @@
+    <div id="overlay-title-wrapper" class="clearfix">
+      <h1 id="overlay-title"<?php print $title_attributes; ?>><?php print $title; ?></h1>

Let's get rid of these superfluous wrapper divs.

+++ b/core/modules/overlay/overlay-child.cssundefined
@@ -21,13 +24,13 @@
 #overlay-titlebar {
-  padding: 0 20px;

Set a solid background color for browsers that can't do hsla()

+++ b/core/modules/overlay/overlay-child.cssundefined
@@ -4,6 +4,9 @@
+/* Import working version from https://drupal.org/node/1945542 */
+@import 'overlay-tabs.css';

Is this temporary? Seems to be an end run around cache

This is starting to shape up already. I do feel like we’re still validating the design (over in the sandbox), so I worry a little about getting into implementation details too early. On the other hand, it might be a good way to see how things integrate? On the whole, I’m happy to see this and happy to help.

One other thing: the style guide intentionally omits breadcrumbs! For some reason we neglected to highlight that fact in the proposal… and possibly no-one noticed?

@gabor Hojtsy

A subtle changes that align better with the intent of the design* (bolded):

#overlay-tabs-secondary li a:active, #overlay-tabs-secondary li.active a {
border-bottom: 2px solid #0074bd;
color: #000;
}

*ry5n please correct me if I'm misinterpreting

From the styleguide, it seems that the tabs don't have overhanging pieces.

The image above is a screenshot from the Seven Styleguide: http://groups.drupal.org/node/283223

I took a shot at the CSS to make it look more like the styleguide.

Bojhan mentioned on IRC that the current style looked bad with Bartik in the background, the blue does not mix well with the overlaid colors.

This patch includes a minor edit to the Bartik CSS to make the theme grayscale when the overlay loads, just as a proof of concept.

Screenshot attached

Bojhan mentioned on IRC that the current style looked bad with Bartik in the background, the blue does not mix well with the overlaid colors.

This patch includes a minor edit to the Bartik CSS to make the theme grayscale when the overlay loads, just as a proof of concept.

Screenshot attached

So as I mentioned in the sandbox issue, I want to propose a simplified version of the tabs. They’ve always felt heavy on the UI chrome, with extra borders and finicky bits. And it’s causing implementation headaches. So here’s something simpler, and I think better:

Observations:
- Far easier to implement
- Looks cleaner
- Is basically what we currently have on drupal.org, so I doubt the more minimal style will cause usability issues.
- Spacing between tabs could be a tiny bit tighter, but I don’t want to waste any more time in Photoshop

I really like the simplification of the visual ornaments in #23.

@ry5n

Looks much better. Great work.

falcon03 asks that we move the tabs inside the #overlay-content div.

Finally, just a question: could we move tabs into the div that the "overlay-content" ID is assigned to?

#1964880-2: Convert the Overlay close button from an anchor element to a button element.

Updated issue summary.

Now part of #1986434: New visual style for Seven - can we get an updated patch for this?

@Gabor Nope, you can push forth with the design now. Me, roy and r5yn agreed upon the direction provided in http://drupal.org/node/1490402#comment-7406260

Raising priority, this will significantly simplify the styling of the modal that still needs to be done and fix a major usability issue with tabs going unnoticed.

Looks beautiful, but I don't see how the breadcrumbs can just be removed like that? (There are tons of admin interfaces in core, and even more in contrib, where you can navigate your way in and then would have no way back to where you were without the breadcrumbs to help you. For previous discussion of this issue, see #548806: No breadcrumbs in Seven.)

Maybe there's a way to add them back but have them be de-emphasized in the design?

David_Rothstein:

I have a design for that. I will find it and add to the issue.

One other thing, I am concerned about the transparency in the header. Visually it's not really as attractive as the original designs *and* depending on what it is on top of it can cause legibility issues. I'm leaning towards wanting to take it out.

+++ b/core/modules/overlay/overlay-child.cssundefined
@@ -26,7 +29,9 @@
+  background: hsla(0, 0%, 42%, 0.65);

We don't have an official standard but it would be good stick to rgba if that's used elsewhere.

+++ b/core/modules/overlay/overlay-child.cssundefined
@@ -26,7 +29,9 @@
+  border-top-left-radius: 5px;
+  border-top-right-radius: 5px;

@@ -35,12 +40,17 @@
+  border-bottom-left-radius: 5px;
+  border-bottom-right-radius: 5px;

We can bring these into one property.

+++ b/core/modules/overlay/templates/overlay.html.twigundefined
@@ -22,15 +22,13 @@
+    <div id="overlay-title-wrapper" class="clearfix">

Let's change this to a class "overlay__title-wrapper"

+++ b/core/modules/overlay/templates/overlay.html.twigundefined
@@ -22,15 +22,13 @@
+    {% if tabs %}<div id="overlay-tabs">{{ tabs }}</div>{% endif %}

This should be a class "tabs--overlay" I think

+++ b/core/modules/shortcut/shortcut.theme.cssundefined
@@ -32,21 +32,21 @@
   background: transparent url(shortcut.png) no-repeat;

We have some icon sprites already in Seven, maybe we should just use those for now?

+++ b/core/themes/seven/style.cssundefined
@@ -229,7 +229,7 @@ pre {
+div[role=tab] ul.primary {

It looks like we've made this selector more specific then it was before, is there a reason for this?

I think once we have the base Seven tabs implemented we'd have a parent varient class, tabs-primary--overlay, similar to the progress bar example. See the changes for .progress-bar__small.

It's hard for me to review the CSS if we are not taking into account all of the new standards. We can split it off into a follow up issue I guess?

Last I checked I had stalled things on account of the tabs. I proposed a new interaction for small screens in Redesign Tabs for Touchscreens.

Assuming we want to use that interaction design, here is a revised style (almost identical to #23) but with all the variants and states taken into account. (If the image below looks confusing, try the prototype, it should make things clearer).

Photoshop source is available: Download PSD

@r5yn I am not sure about this. The earlier design has a lot more hierarchy, that is now lost when you make the options blend in with the background. The only thing I would focus on improving is the indicator you have opened the tray. Also for sake of progress, could we get a first iteration in? This is a fundamental improvement, and I am afraid if we keep iterating endlessly in this thread no one will have the idea that we can move parts of this in.

The gradient is a bit misleading, obviously we do not want there to actually be a gradient.

Yes, no gradient. Which version do you want to go with? EDIT: I assume you mean the one in the issue summary.

After considering it, I’m OK with moving forward with the original design for the sake of progress (unless a bunch of people express a preference).

@ry5n

I would not give up so quickly on this one. This is a simplification that removes quite a bit of visual noise from the UI with zero usability loss as far as I can see. You have my vote for pushing forward on it, especially since it only involves removing—not adding—code.

Awesome work.

-Kevin

I think the updated tabs design is better, but it does not sound like we have consensus on that. Gábor is right: let’s get this moving again, and hopefully we have time to make a choice later about the tabs.

meta issue added

@ry5n

Sounds sensible.

BTW Jesse beach just saw your tabs prototype and pronounced it "damn sweet"

@Gábor Hojtsy

I think this is very close to being done. I only see a couple of things that need to be changed.

/* Remove the active state background from the secondary tabs */
#overlay-tabs ul.secondary a
- border-bottom: 1px solid transparent;
+ border-bottom: 2px solid transparent;
+ background: transparent;
}

And if we want to remove the styling on inactive tabs as in Ryan's design:

/* Remove the background from the parent */
#overlay-tabs {
- background-color: #ffffff;
+ background: transparent;
}

/* Remove the borders from the primary tabs */
#overlay-tabs ul.primary a {
- border-top: 1px solid #b3b3b3;
- border-left: 1px solid #b3b3b3;
}

/* Remove the background color and borders from the containing div */
#overlay-tabs ul.primary {
- background-color: #f2f2f0;
- border-radius: 5px 5px 0 0;
- border-top: 1px solid #b3b3b3;
}

/* remove the background color from the inactive tab and change the link color to contrast with the header background */
#overlay-tabs ul.primary a:hover {
- background-color: #fff;
+ background: transparent;
- color: #007ac7;
+ color: #8DD2FF;
}

/* add a hover color for the primary links and make it fade on */
#overlay-tabs ul.primary a:hover {
+ color: #fff;
+ -webkit-transition: all 1s;
}

/* add back white background but to secondary links only */
#overlay-tabs ul.secondary {
+ background-color: #ffffff;
}

Kevin could you provide the icons?

@Gábor Hojtsy

Ah. I think I missed a couple of attributes.

#overlay-tabs ul.primary .active ~ li a {
- border-left: 0;
- border-right: 1px solid #b3b3b3;
- border-radius: 0 5px 0 0;
}

#overlay-tabs ul.primary {
+ padding-left: 1em;
}

#overlay-tabs ul.primary a {
- color: #0074bd;
+ color: #8DD2FF;
}

The active and hove colors in the secondary tabs match the style guide.

And this:

#overlay-tabs ul.primary .active a {
- border-right: 1px solid #b3b3b3;
- -webkit-box-shadow: 2px -1px 2px -1px rgba(0, 0, 0, 0.2), -2px -1px 2px -1px rgba(0, 0, 0, 0.2);
- -moz-box-shadow: 2px -1px 2px -1px rgba(0, 0, 0, 0.2), -2px -1px 2px -1px rgba(0, 0, 0, 0.2);
- box-shadow: 2px -1px 2px -1px rgba(0, 0, 0, 0.2), -2px -1px 2px -1px rgba(0, 0, 0, 0.2);
}

@Gábor Hojtsy

I see. I can fix that. I need to add a first child element.

Darn images dont load on my phone, probally wont be able to review till I am back from vacation. Not sure about the struggels the redesign should be pretty clear, if you need a clear updated psd i am sure that can happen.

#66: overlay-restyle-66.patch queued for re-testing.

Quick reroll.

Looking gorgeous! Only found this glitch noted in the 1st screenshot. Only added margin: 0; padding: 0; to #overlay-tabs to resolve, 2nd screenshot.

Also, notice in the patches there's a pretty big rdf tag for the images, looks like added by the graphics program. Never seen this in our patches before, I would think we need to save the images without it.

This is a tricky patch, it relies on components that have been implemented from #1490402: Redesign tabs and the content header and #2022695: Content header style update. In this issue we've actually re-implemented components from both of those sections.

I've tagged all these issues with 'styleguide-navigation', just to group them for now, it's likely they will be committed together as one patch. I've created a Seven Style Guide Initiative sandbox that includes all of the style guide issue patches. It also includes a branch that merges together all three navigation issues. I've created a patch from that branch against 8.x. It's totally broken, but hopefully it's a better base to work from.

Ok, so the RTL patch of doom knocked everything for six. Prior to that I experimented with the theming for the overlay tabs and realised I could get them looking pretty good without any custom styling. There is still work to do but it's not nearly as much as we were doing before.

The sandbox branch this is based off is: navigation-2022685-1490402-1953374

hm, I think I forgot to attach the patch with the last post... I had to remerge everything again anyway

Still some work to do here, most noticeably the add to favourites icon.

@Lewis I am reviewing this, am I correct the tabs are not styled like the style guide?

All other aspects look fine, the only thing I am contemplating whether we should ditch the transparency and just have a gray top.

@bojhan A lot of the work to get to the latest patch was to merge the overlay work with the latest patch in #1490402: Redesign tabs and the content header. I removed all overlay specific tabs styling to give us a better view of what we need to add in. We aren't that far off

You might need to be more specific when referencing the style guide, I'm not sure if we have a complete canonical version anymore, the most recent designs are in this issue, we should be working towards that right?

Here's an attempt at a re-roll.

@LewisNyman

Looking pretty good. Some small issues:

Drop-button overflow is hidden
Responsive tabs not working correctly
Border-left on first child tab looks odd

Doing a bit of housekeeping on the Spark queue in order to re-familiarize myself with it, and this doesn't look strictly related to me. Tentatively removing tag.

@LewisNyman

Bug from #82 is fixed ( ... no longer kicks up to the top right) but the tabs still break at under 320px. Try using the elipsis then tabbing to a different tab.

@tkolery I don't consider that bug, the overlay should never open for mobile users, opening it in a desktop browser and resizing to a mobile width isn't a use case I considered supporting, I'm open to being convinced though, if you think it is a genuine use case.

@LewisNyman

Right. Good point. I should be testing in xcode.

updated related issues for meta

Going to mark this RTBC. Looks like Kevin his issues are resolved in all the other issues.

#81: overlay-restyle-1953374-81.patch queued for re-testing.

Doesn't look like the patch applies... but also, does this still have the breadcrumb problem (see #36 and #37)?

The last patch was mega old. I've re-rolled it against the patch of #1490402: Redesign tabs and the content header #2022695: Content header style update.

Screenies:

I wonder if it's worth restoring the breadcrumbs and splitting the discussion about removing them into a separate issue. I'd hate to see this really nice styling get held up by a re-hash of the discussion that already happened in #548806: No breadcrumbs in Seven about this in D7. (The original version of Seven also intentionally didn't have breadcrumbs, for probbaly the same reasons as the Seven Style guide folks want to remove them, but it was deemed too difficult to navigate out of most admin pages which tend to be buried 2-3 levels deep.)

Agreed, although you could argue that the new toolbar fills that role, I'd rather that discussion did not block these three issues. I'll reopen #2022695: Content header style update.

Bojhan said that he would be happy for the breadcrumbs to sit below the secondary tabs for now, which would be nice as we wouldn't have to go back to design to figure out how to accommodate them in the content header.

Added tabs

Forgot to close this http://media1.giphy.com/media/n5NHEm5gkuml2/giphy.gif