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.

Proposed resolution

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

Remaining tasks

User interface changes

API changes

CommentFileSizeAuthor
#17 after.png44.14 KBrudraram
#17 before.png44.51 KBrudraram
#13 Welcome_to_test___test.png94.02 KBzetagraph
#10 use_underline_as_the-2487704-8.patch2.07 KBprospekt
#5 drupal-focus-states-admin-2487704.patch790 bytesprospekt
#2 Screen Shot 2015-05-13 at 07.36.14.png11.01 KBBojhan
#2 Screen Shot 2015-05-13 at 07.37.13.png16.52 KBBojhan
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

mark.labrecque’s picture

mark.labrecque
he/him
Primary language English
Location Vancouver
CreditAttribution: mark.labrecque at ImageX commented

I would be happy to tackle this, but could I get some more information describing what is the desired effect here? Perhaps some screenshots might be helpful here?

Thanks!

Bojhan’s picture

Bojhan CreditAttribution: Bojhan as a volunteer commented
FileSize
Screen Shot 2015-05-13 at 07.37.13.png16.52 KB
Screen Shot 2015-05-13 at 07.36.14.png11.01 KB


Is what I am proposing. If you look at Drupal now you will see at odd places we have a border-left or border-top. We want that to be underline on the text for the focused state.

LewisNyman’s picture

LewisNyman
he/him
CreditAttribution: LewisNyman at Wunder commented

Here is an example from Seven's elements.css 

details summary:focus {
  border-top: 3px solid #0074bd;
  outline: none;
  color: #0074bd;
  margin-top: -3px;
}
prospekt’s picture

prospekt CreditAttribution: prospekt as a volunteer commented

Currently at the Drupalcon LA core sprint - I'm going to take a look at this issue considering it hasn't been updated for two days.

prospekt’s picture

prospekt CreditAttribution: prospekt as a volunteer commented
FileSize
drupal-focus-states-admin-2487704.patch790 bytes

I've uploaded a patch that adds some underline focus states, but we may need a wider sweep of the seven theme to see where some other focus states are missing.

Bojhan’s picture

Bojhan CreditAttribution: Bojhan as a volunteer commented

@AlexB Your at the sprint? Lets review - I am at the frontend table.

prospekt’s picture

prospekt CreditAttribution: prospekt as a volunteer commented

I'm at one of the first timer tables in 403a. I'll figure out where you are though and come by in a minute.

Bojhan’s picture

Bojhan CreditAttribution: Bojhan as a volunteer commented
Component: Seven theme » toolbar.module

Lets first tackle the Toolbar. I will make a separate issue for Seven.

Bojhan’s picture

Bojhan CreditAttribution: Bojhan as a volunteer commented

#2489450: Remove unnecessary focused/hover effects on details and vertical tabs

prospekt’s picture

prospekt CreditAttribution: prospekt as a volunteer commented
Status: Active » Needs review
FileSize
use_underline_as_the-2487704-8.patch2.07 KB

Added more focus styles to the toolbar menu and removed unnecessary borders as well.

Status: Needs review » Needs work

The last submitted patch, 10: use_underline_as_the-2487704-8.patch, failed testing.

Status: Needs work » Needs review

Bojhan queued 10: use_underline_as_the-2487704-8.patch for re-testing.

zetagraph’s picture

zetagraph CreditAttribution: zetagraph commented
FileSize
Welcome_to_test___test.png94.02 KB

Tested. Border is replaced with underline on focus.

screenshot-of-focus-test

lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
CreditAttribution: lauriii at Druid commented

I personally liked the border style so I'm sad to see it being taken away :(

zetagraph’s picture

zetagraph CreditAttribution: zetagraph commented
Status: Needs review » Reviewed & tested by the community
webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick at Acquia commented
Status: Reviewed & tested by the community » Needs review
Issue tags: +Needs screenshots

Hm. Can we maybe get some before/after screenshots here or something, please? To my untrained eye, the toolbar is already showing an underline without this patch on hover. And I don't see any borders?

rudraram’s picture

rudraram CreditAttribution: rudraram at Axelerant commented
FileSize
before.png44.51 KB
after.png44.14 KB

Attaching before and after screenshots.

Before:
before

After:
after

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick at Acquia commented
Status: Needs review » Fixed

Thanks a lot, rudaram! That's much clearer.

Like laurii I actually kinda prefer the way it currently is, but I can see points for consistency.

Ergo, committed and pushed to 8.0.x. Thanks!

  • webchick committed 223f92b on 8.0.x 
    Issue #2487704 by AlexB-appnovation, Bojhan, rudraram, zetagraph: Use...

  • webchick committed 7f541ef on 8.0.x 
    Revert "Issue #2487704 by AlexB-appnovation, Bojhan, rudraram, zetagraph...

  • webchick committed d81c986 on 8.0.x 
    Issue #2487704 by AlexB-appnovation, Bojhan, rudraram, zetagraph: Use...
webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick at Acquia commented

Sorry, I accidentally committed the responsive image UI patch along with this one.

Status: Fixed » Closed (fixed)

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