Problem/Motivation

We currently use many different ways to hover/focus on items. Many of these styles were introduced in the last few months. Sadly this was not in alignment with the style guide.

For the style guide we propose for all text elements to have the "underline" as focused style. This means fieldsets, vertical tabs, toolbar links, etc.

This issue focuses on removing these unnecessary effects from Seven.

Proposed resolution

  • Use underline as focused style (were appropriate as hover too)

Remaining tasks

User interface changes

API changes

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

jibla’s picture

I am in LA DrupalCon and going to fix this issue.

Bojhan’s picture

See #2487704: Use underline as the focused state (not border left/bottom). We are fixing toolbar atm. So this really contains to fixing the hover/focused styles of fieldsets/vertical tabs.

jibla’s picture

Does this issue mean that we should remove all styles but text-decoration: underline for ALL :focused/:hover styles?
I mean for example we have in admin-list.css:
.button:hover,
.button:focus {
background-color: #f9f8f6;
background-image: -webkit-linear-gradient(top, #fcfcfa, #e9e9dd);
background-image: linear-gradient(to bottom, #fcfcfa, #e9e9dd);
color: #1a1a1a;
text-decoration: none;
outline: none;
}

should I change it like this?
.button:hover,
.button:focus {
text-decoration: underline;
}
?

or should I change only text-decoration, like this?
.button:hover,
.button:focus {
background-color: #f9f8f6;
background-image: -webkit-linear-gradient(top, #fcfcfa, #e9e9dd);
background-image: linear-gradient(to bottom, #fcfcfa, #e9e9dd);
color: #1a1a1a;
text-decoration: underline;
outline: none;
}

Bojhan’s picture

What we want to do is evaluate the places were we add a border as hover effect. This is on elements such as fieldsets, vertical tabs. What we want to do instead of adding a border-top or border-left. Is to add "underline" on the main label. This underline is only triggered by hover or focused.

jibla’s picture

Status: Active » Needs review
FileSize
10.06 KB
jibla’s picture

Status: Needs review » Active
matthodgson’s picture

I'll begin reviewing this patch.

matthodgson’s picture

I was able to review the changes to tables.css.

This needs to be removed:

+++ b/core/themes/seven/css/components/tables.css
@@ -97,13 +100,14 @@ th > a:focus,
   border-bottom-color: #008ee6;

I believe this needs to be removed from the original file as well:

th.is-active > a:after {
  border-bottom-color: #004875;
}

I will try to review more of the patch after leaving drupalcon la.

cmanalansan’s picture

Sprinting at DrupalCon LA, I'll start looking at this!

LewisNyman’s picture

Issue tags: +frontend, +CSS
cmanalansan’s picture

Going to dinner.

I was going to upload a patch.

I swear I was.

kfitz’s picture

I was able to review the changes to tables.css.

This needs to be removed:

+++ b/core/themes/seven/css/components/tables.css
@@ -97,13 +100,14 @@ th > a:focus,
border-bottom-color: #008ee6;
I believe this needs to be removed from the original file as well:

th.is-active > a:after {
border-bottom-color: #004875;
}
I will try to review more of the patch after leaving drupalcon la.

Made the changes to the patch according to the comments above

kfitz’s picture

Status: Active » Needs review
maartendeblock’s picture

Status: Needs review » Reviewed & tested by the community

Looking good!

LewisNyman’s picture

Issue summary: View changes
Status: Reviewed & tested by the community » Needs review
FileSize
41.89 KB
41.89 KB
10.88 KB

I'm not sure about the styling for the admin panels. Does anyone else think the styling for the admin panel and the buttons look quite bad? I don't think these are part of the original scope of the issue.

Bojhan’s picture

It should only be on text links, not buttons or anything else. There is no reason the underline should ever be triggered on descriptions.

Also is this making titles in tables not blue? This patch could probably use some handholding (e.g. developer next to designer).

manauwarsheikh’s picture

Status: Needs review » Needs work
FileSize
52.28 KB

The underlined appearance should not be applicable for "buttoned links", It should be applicable for links in the description text.Also there is extra hover on desription when "webmaster" hover on any links on the configuration page(attached screenshot).

Next Steps: commenting/removing extra "underline" css.

marieke_h’s picture

Assigned: Unassigned » marieke_h

I'll look into this.

marieke_h’s picture

Assigned: marieke_h » Unassigned
Status: Needs work » Needs review
FileSize
1.4 KB
9.48 KB

I updated the theming for admin-list hover, to only underline the label.
And also fixed the buttons.

pjbaert’s picture

Issue tags: +Barcelona2015

added tag.

LewisNyman’s picture

Status: Needs review » Needs work

I don't understand why this patch is adding a load of text-decoration: underline effects.

Going back to comment #4, all we want to do is remove the border focus effects from the details and verticals and replace it with text underline.

LewisNyman’s picture

Issue summary: View changes
Status: Needs work » Needs review
Issue tags: -Novice
FileSize
1.69 KB
77.84 KB
22.61 KB

Here's a new patch that only changes styling for the details elements and vertical tabs.

Screenshots:

Bojhan’s picture

@Lewis There are many other places (e.g. toolbar) is that also part of this issue or should we open a separate for that?

Manjit.Singh’s picture

@Bojhan If that is the case then we should create sub-tasks or child issues of this. Otherwise it would be bit difficult to check regression issues on many other pages.

LewisNyman’s picture

The toolbar doesn't have these border effects on focus any more

Bojhan’s picture

Title: Remove unnecessary focused/hover effects » Remove unnecessary focused/hover effects on details and vertical tabs
Status: Needs review » Reviewed & tested by the community
jcnventura’s picture

One very small change, as I felt the underline on details summary to have inconsistent behaviour.

I understand it needs to be kept underlined for focused, but it felt weird to not have it on hover. This patch corrects that, bringing it inline with the normal behaviour in the rest of the admin interface.

Sorry for setting it back, but other this minor annoyance, I'd have +1 the RTBC.

LewisNyman’s picture

Status: Needs review » Reviewed & tested by the community

Makes sense, we do this in some areas, like under admin/config and the toolbar. This feels consistent. Thanks.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 27: remove_unnecessary-2489450-27.patch, failed testing.

jcnventura’s picture

Status: Needs work » Reviewed & tested by the community

Stupid bot

alexpott’s picture

Version: 8.0.x-dev » 8.1.x-dev
Status: Reviewed & tested by the community » Fixed

Given that this in not fixing a bug this can be committed to 8.1.x. Committed cbca117 and pushed to 8.1.x. Thanks!

  • alexpott committed cbca117 on 8.1.x
    Issue #2489450 by LewisNyman, marieke_h, kfitz, jcnventura, jibla,...

Status: Fixed » Closed (fixed)

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