It seems to be pretty jarring to me atleast when I hover over or click on buttons in the seven theme.

I think there needs to be some middle ground for when the buttons are being hovered over in order to make the transition seem smoother.

Files: 
CommentFileSizeAuthor
#57 seven-buttons-hover-846182-57.patch5.31 KBgeerlingguy
PASSED: [[SimpleTest]]: [MySQL] 41,087 pass(es).
[ View ]
#54 ButtonOnFocus.png19.76 KBmgifford
#54 ButtonOffFocus.png14.85 KBmgifford
#52 seven-buttons-hover-846182-52.patch5.28 KBgeerlingguy
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch seven-buttons-hover-846182-52.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#47 seven-buttons-hover-846182-47.patch5.62 KBreglogge
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch seven-buttons-hover-846182-47.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#47 screen-buttons-seven.png6.5 KBreglogge
#38 buttons.png6.5 KBreglogge
#32 seven-buttons-hover-32.patch3.2 KBreglogge
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch seven-buttons-hover-32.patch. This may be a -p0 (old style) patch, which is no longer supported by the testbots.
[ View ]
#29 screen-buttons-grey.png3.17 KBreglogge
#29 configure-dark.png367 bytesreglogge
#29 seven-buttons-grey.png503 bytesreglogge
#29 seven-buttons-grey-1.patch3.22 KBreglogge
PASSED: [[SimpleTest]]: [MySQL] 27,413 pass(es).
[ View ]
#28 Seven-theme-issue-846182.png3.02 KBadrinux
#28 Seven-theme-issue-846182-grey.png2.43 KBadrinux
#21 screen-buttons-grey.png10.71 KBreglogge
#21 seven-buttons-grey.patch2.34 KBreglogge
PASSED: [[SimpleTest]]: [MySQL] 26,171 pass(es).
[ View ]
#21 seven-buttons-grey.png503 bytesreglogge
#20 screen-buttons-blue.png11.44 KBreglogge
#20 seven-buttons-blue.patch2.21 KBreglogge
PASSED: [[SimpleTest]]: [MySQL] 25,216 pass(es).
[ View ]
#20 seven-buttons-blue.png1.08 KBreglogge
#19 markup_test.zip15.27 KBJacine
#18 seven-buttons-hover-grey.png6.31 KBreglogge
#18 seven-buttons-hover-grey.patch2.29 KBreglogge
PASSED: [[SimpleTest]]: [MySQL] 25,215 pass(es).
[ View ]
#18 buttons.png503 bytesreglogge
#15 seven-buttons-hover.png11.92 KBreglogge
#14 seven-buttons-hover-2.patch1.68 KBreglogge
PASSED: [[SimpleTest]]: [MySQL] 25,204 pass(es).
[ View ]
#13 seven-buttons-hover-1.patch1.07 KBreglogge
FAILED: [[SimpleTest]]: [MySQL] Setup environment: failed to drop checkout database.
[ View ]
#8 seven-buttons-hover.patch1.07 KBreglogge
PASSED: [[SimpleTest]]: [MySQL] 25,189 pass(es).
[ View ]
#8 seven-buttons-hover.png16.39 KBreglogge
#6 style.patch609 bytesszantog
PASSED: [[SimpleTest]]: [MySQL] 25,169 pass(es).
[ View ]
#3 style.patch607 bytesszantog
PASSED: [[SimpleTest]]: [MySQL] 24,830 pass(es).
[ View ]
#1 style.patch621 bytesszantog
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch style_17.patch.
[ View ]

Comments

szantog’s picture

Status:Active» Needs review
StatusFileSize
new621 bytes
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch style_17.patch.
[ View ]

This patch set the active grey background on hover state.

Status:Needs review» Needs work

The last submitted patch, style.patch, failed testing.

szantog’s picture

StatusFileSize
new607 bytes
PASSED: [[SimpleTest]]: [MySQL] 24,830 pass(es).
[ View ]

Hmm.. Sorry, a hope, this is correct patch format.

szantog’s picture

Status:Needs work» Needs review

and let's needs review..

Jeff Burnz’s picture

Status:Needs review» Needs work
+++ themes/seven/style.css 23 Sep 2010 17:56:14 -0000
@@ -673,7 +673,7 @@
+input.form-submit:active, input.form-submit:hover {

Selectors all go on new lines to conform to Drupal CSS coding standards.

Powered by Dreditor.

szantog’s picture

Status:Needs work» Needs review
StatusFileSize
new609 bytes
PASSED: [[SimpleTest]]: [MySQL] 25,169 pass(es).
[ View ]

Ok, I learned it. :)

mikey_p’s picture

It seems that it would be nice to have two separate states for hover and active. Is there anywhere else that does this in core?

reglogge’s picture

StatusFileSize
new16.39 KB
new1.07 KB
PASSED: [[SimpleTest]]: [MySQL] 25,189 pass(es).
[ View ]

Since the button-background in seven is already set to a slice with grey and blue backgrounds, it would be better to just use this. We can activate the lower half of the background-image in blue for the hover-state (screenshot and patch attached).

The :active styling remains the same as before (dark grey background).

When using the :hover pseudo-class we should also always use :focus for the folks who use the keyboard to navigate.

The styles with div.node-form input#edit-submit look like cruft, since on the node form there is no div.node-form anymore...

Jeff Burnz’s picture

I always wondered why that blue bit was not used! This needs broader reviews.

Jacine’s picture

Status:Needs review» Needs work

Cool! This will need to be updated in jquery.ui.theme.css too. Those buttons are styled for Dialog boxes (not the overlay).

reglogge’s picture

@jacine: where are the dialog boxes used in core? I can't find a place to test them.

Jacine’s picture

I'll get you some to test this with soon, since there is nothing in core. Hang tight :D

reglogge’s picture

StatusFileSize
new1.07 KB
FAILED: [[SimpleTest]]: [MySQL] Setup environment: failed to drop checkout database.
[ View ]

Adjusted the text-color of the :hover state button a little so the buttons pass WCAG AA and WCAG AAA. I tested this with a background-color of #98c4df, which is right in the middle of the blue background-image. Test run here: http://webaim.org/resources/contrastchecker/

reglogge’s picture

Status:Needs work» Needs review
StatusFileSize
new1.68 KB
PASSED: [[SimpleTest]]: [MySQL] 25,204 pass(es).
[ View ]

Sorry, but the patch in #13 was borked. New, correct patch with the styles also applied in jquery.ui.theme.css.

EDIT: I don't have a way to test/preview the buttons in a jquery control. Jacine will test this over the weekend :-)

reglogge’s picture

StatusFileSize
new11.92 KB

Screenshot of all three button states (normal, :hover/:focus, :active) attached

Jeff Burnz’s picture

Looking good, the only thing I am not sure about is the blue, because everywhere else Seven (and Toolbar) are shades of gray, should we be introducing a color here or use another warm gray instead?

reglogge’s picture

I'm not sure either. The blue was just there already, so I assumed that it was meant to be used...

To me it's rather pretty though.

reglogge’s picture

StatusFileSize
new503 bytes
new2.29 KB
PASSED: [[SimpleTest]]: [MySQL] 25,215 pass(es).
[ View ]
new6.31 KB

Alternative in grey with the :active state button also having his background from the slice.

Patch and the new background-slice attached as well as a screenshot.

All colors pass WCAG AA and AAA.

buttons.png has already been smushed to 503 bytes.

Jacine’s picture

StatusFileSize
new15.27 KB

@reglogge I didn't get a chance to test today, and probably wont be able to until Monday, but I DID get you some code to test with. You can either check it out via CVS: "contributions/sandbox/sun/markup_test" or use the tarball I've attached. Once enabled go to markup_test/page/theme-roller.

BTW, don't blame me for those tabs because that's not the code I got committed. :D Not sure what happened there.

I hope this helps!

reglogge’s picture

StatusFileSize
new1.08 KB
new2.21 KB
PASSED: [[SimpleTest]]: [MySQL] 25,216 pass(es).
[ View ]
new11.44 KB

@jacine: Thanks, that really helps!

So here is the entire shebang, tested with regular buttons and also in jquery-dialogs.

Two versions: blue and grey hover states.

The blue variant (in this comment) consists of:
- The patch seven-buttons-blue.patch
- The screenshot screen-buttons-blue.png
- The new background-image seven-buttons-blue.png that needs to go into sites/all/themes/seven/images and be renamed to just buttons.png there

The grey variant (in the next comment) consists of:
- The patch seven-buttons-grey.patch
- The screenshot screen-buttons-grey.png
- The new background-image seven-buttons-grey.png that needs to go into sites/all/themes/seven/images and be renamed to just buttons.png there

For both variants I now use a new background-image for all the button states.
In the active state, the dark button gets a slightly less dark border to underline its *inverted* character (it has white text on dark background). Feel free to discuss this :-)

All colors in both patches pass WCAG AA and AAA.

reglogge’s picture

StatusFileSize
new503 bytes
new2.34 KB
PASSED: [[SimpleTest]]: [MySQL] 26,171 pass(es).
[ View ]
new10.71 KB

Grey variant as described in #20

mgifford’s picture

I like it when it's in hover mode, I don't like how it looks when you click on it.

mgifford’s picture

#21: seven-buttons-grey.patch queued for re-testing.

mgifford’s picture

I think @Bojhan or @yoroy need to give some usability feedback on this I think it's an improvement, but some design folks associated with Seven need to give it the thumbs up I think.

Jeff Burnz’s picture

yes I think its an improvement also, blue seems be the color of choice for "message display" such as skip link (will likely change to blue), the "show blocks back to the blocks page" reverse tab and the new "disable the overlay" messages. I don't even like the blue hover state at all, it just seems to jump out of nowhere for no apparent reason - the grays are much nicer.

mikey_p’s picture

This looks good to me, I'll try to post a video of this shortly.

mikey_p’s picture

Status:Needs review» Reviewed & tested by the community

Couldn't post a video, but fwiw, Bartik has a similar hover state, but no active styles :(.

adrinux’s picture

Status:Reviewed & tested by the community» Needs work
StatusFileSize
new2.43 KB
new3.02 KB

I found this issue after getting frustrated with the 'manage display' tab when editing a content type - took me ages to figure out I had to use the button with a cog because it looked greyed out and didn't respond on hover - the latter problem is part of this issue.

Patch in #21 does indeed improve most buttons but not these buttons with a cog. Attached images show that it looks bad in blue but worse in grey - unfortunately the background colour chosen by reglogge in seven-buttons-grey.png matches the colour used for the cog pretty closely.

That said, I don't think I'm the only one that thinks the default state of these cog buttons looks 'greyed out'. So perhaps the solution is to use a darker cog.
Currently these buttons are using the core misc/configure.png for the cog. It might be easier to use seven's own cog in ui-icons-222222-256x240.png or ui-icons-454545-256x240.png, but that's also much smaller circa 14px vs 6px. So not a good replacement.

Or perhaps those cog buttons need a different hover background colour - but that seems inconsistent.

Not sure which way to go, but patch needs work :(

reglogge’s picture

Status:Needs work» Needs review
StatusFileSize
new3.22 KB
PASSED: [[SimpleTest]]: [MySQL] 27,413 pass(es).
[ View ]
new503 bytes
new367 bytes
new3.17 KB

Here is a new patch based on #21 using a new darker image for the cog.

- The attached image configure_dark.png needs to go into /misc
- The attached image seven-buttons-grey.png needs to go into /themes/seven/images and to be renamed to buttons.png there
- The attached screenshot screen-buttons-grey.png shown the normal and hover states of the button with a cog side by side

joachim’s picture

Looks great, and the fix to the display settings cog is very good too.

But the disabled buttons still change when I hover, which makes them look like they *might* be clickable after all.

joachim’s picture

One other thing, the buttons now look pretty weird on click.

reglogge’s picture

Priority:Normal» Major
StatusFileSize
new3.2 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch seven-buttons-hover-32.patch. This may be a -p0 (old style) patch, which is no longer supported by the testbots.
[ View ]

joachim: Could you be more specific as to which "disabled" buttons still change state on :hover?

I attached a patch that darkens the border for the buttons on the :active state, so that they don't look inverted anymore.

The additional files from #29 would still have to be committed additionally.

Also bumping this to major since missing hover states are pretty bad for accessibility reasons. I think this should get in before release.

joachim’s picture

> joachim: Could you be more specific as to which "disabled" buttons still change state on :hover?

Argh, I can't find any disabled buttons in the UI :/

Josh The Geek’s picture

Subscribing. Looks good!

adrinux’s picture

@joachim I can't find any disabled buttons either - in fact I don't think there are any, I think the problem may be that buttons you thought were greyed out actually weren't, they *looked* disabled, but actually did something. After all if it's disabled, ie does nothing, why bother printing it at all?

yoroy’s picture

Issue tags:+Usability

Tagging. Will test this later but could use screenshots 'in action'.

adrinux’s picture

@yoroy doesn't screen-buttons-grey.png in #29 do the job? It's an amalgamated screenshot showing both states, not an image that needs adding to the theme.

reglogge’s picture

StatusFileSize
new6.5 KB

Here is a composite screenshot of the buttons and the cog-buttons in their state from #32

dixon_’s picture

Assigned:Unassigned» dixon_

This seems to be able to still get into D7?

The Drupal 7 release party in Stockholm, Sweden will update, test and review this patch so we hopefully can make some progress on it.

By assigning this to me, I'm by no means stealing this issue. Just stating that I'll coordinate some work on this tonight.

reglogge’s picture

@dixon_: Are you still working on this?

dixon_’s picture

Assigned:dixon_» Unassigned

Sorry, not really. BI tried to organize some work around this on our D7 release party. But the party is over now.

But I'd gladly take on the challange later, though! :)

Jeff Burnz’s picture

Version:7.x-dev» 8.x-dev
Status:Needs review» Needs work
Issue tags:+#d8ux

Can we get a re-roll of the latest patch here - the one in #32. UX definitely needs to look at this.

quicksketch’s picture

Priority:Major» Normal

I should note that most browsers don't have a "hover" state on default buttons at all. This doesn't seems like a bug at all. The lack of an "active" state (the state when the mouse button is down) is quite annoying however. The discussion in #1050616: Figure out backport workflow from Drupal 8 to Drupal 7 looks as if it's going to make major and critical bugs actually prevent development of new features in Drupal core, so I'm re-categorizing issues to their proper levels (see http://drupal.org/node/45111).

Bojhan’s picture

Sooo many pictures, can we get the final in-action pictures of the hover state?

I feel this is pretty close to RTBC though.

mgifford’s picture

Status:Needs work» Needs review
Issue tags:-Usability, -Needs design review, -needs accessibility review, -#d8ux

#32: seven-buttons-hover-32.patch queued for re-testing.

Status:Needs review» Needs work
Issue tags:+Usability, +Needs design review, +needs accessibility review, +#d8ux

The last submitted patch, seven-buttons-hover-32.patch, failed testing.

reglogge’s picture

Status:Needs work» Needs review
StatusFileSize
new6.5 KB
new5.62 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch seven-buttons-hover-846182-47.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Here's a new patch that should apply.

I also re-attached the screenshot from #38 that shows the buttons in normal, hover and active state as well as the new darkened cog which also works on buttons in hover state (screen-buttons-seven.png).

mgifford’s picture

I tried to port this to the new file layout. Unfortunately Hunk #1 FAILED at 657.
1 out of 1 hunk FAILED for rejects to file core/themes/seven/style.css

So this needs a bit more time.

Bojhan’s picture

Issue tags:-Needs design review

Looks good to me visually, so feel free to put this to RTBC

mgifford’s picture

Status:Needs review» Needs work
Issue tags:+Usability, +needs accessibility review, +#d8ux

The last submitted patch, seven-buttons-hover-846182-47.patch, failed testing.

geerlingguy’s picture

Status:Needs work» Needs review
StatusFileSize
new5.28 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch seven-buttons-hover-846182-52.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Manual re-roll from #47.

nod_’s picture

Status:Needs review» Reviewed & tested by the community

tested and works well.

mgifford’s picture

StatusFileSize
new14.85 KB
new19.76 KB

Agreed. I ran it through the color checker too and that went fine.
http://webaim.org/resources/contrastchecker/

Will be great to get this in Core.

catch’s picture

Status:Reviewed & tested by the community» Needs work
Issue tags:+Usability, +needs accessibility review, +#d8ux

The last submitted patch, seven-buttons-hover-846182-52.patch, failed testing.

geerlingguy’s picture

Status:Needs work» Needs review
StatusFileSize
new5.31 KB
PASSED: [[SimpleTest]]: [MySQL] 41,087 pass(es).
[ View ]

Re-roll (the file core/themes/seven/jquery.ui.theme.css was changed at some point to remove the vendor prefixes). Should be good to go again, but letting testbot run first.

geerlingguy’s picture

Status:Needs review» Reviewed & tested by the community

Back to RTBC, since there were no code changes, just a reroll.

webchick’s picture

Status:Reviewed & tested by the community» Fixed

Nice visual improvement! Also, I'm shocked that you can get graphics like that into a patch. Let's see if it works. ;)

Committed and pushed to 8.x. Thanks!

geerlingguy’s picture

Nice! And, I've had great success doing git add to stage new files (and changes), then running git diff --cached --binary to get all the changes (including new files) in one patch file. Makes life easier when you don't have to throw around a bunch of files along with a patch.

Bojhan’s picture

Status:Fixed» Active

Hmmpf, lets discuss this some more. Having it seen in action, its really a bit strong - we shouldn't really be using a plain color, where the rest uses a elegant gradient.

I know I signed of on it, but I was wrong :) Luckily we aren't in release mode, so lets just work on this some more.

mgifford’s picture

@geerlingguy can you revise the patch (realizing the old one is now in core) to consider @Bojhan requirements.

Any concerns with reducing the contrast a bit so it isn't quite so strong?

Need any help to add in the gradient?

I really wish that for Seven that we had a Inkscape.org vector image that anyone could just download and muck with. I'm not sure how your original images were done, but would be useful to share them if you can.

geerlingguy’s picture

I think I was using the images from #29 - maybe reglogge has the originals?

Bojhan’s picture

Assigned:Unassigned» Bojhan

I will take the time to come up with good alternatives and try to WCAG check em

LewisNyman’s picture

I'm going to try and find time to come up with a follow up patch for this that makes the active states easier on the eye, this is really bothering me.

Bojhan if you want to find time to throw ideas around and do some quick iterations let me know.

Bojhan’s picture

I am actually working on this, however I decided to scope it to everything in the Seven world.

LewisNyman’s picture

Status:Active» Fixed

Let's mark this as 'fixed', this issue has a spiritual follow up in #1986074: Buttons style update

Status:Fixed» Closed (fixed)
Issue tags:-Usability, -needs accessibility review, -#d8ux

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