The standard Button, Dropdown Button and Splitbutton have been implemented (see http://drupalcode.org/sandbox/ry5n/1932040.git/blob_plain/HEAD:/index.html). However, we need to add the Primary and Delete variants, and the Small variant (which should work when applied to any button). For reference, see http://groups.drupal.org/node/283223#Buttons and http://groups.drupal.org/node/283223#Small_Controls.

We may only need these two new variants for the .button class:

.button--primary
.button--small

The Delete button is styled as a link. Rather than define it as .button--delete, it is probably better to create a .link class and style is as .link--danger. So, danger buttons would be marked up as <button class="link link--danger">Async delete</button> or <a href="" class="link--danger" href="">Delete node</a>

CommentFileSizeAuthor
#6 buttons-p5.txt6.69 KBry5n
#6 buttons-p4-with-p5.interdiff.txt8.24 KBry5n
#4 buttons-4.patch4.73 KBLewisNyman
#1 buttons-1.patch5.14 KBLewisNyman
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

LewisNyman’s picture

LewisNyman
he/him
CreditAttribution: LewisNyman commented
FileSize
buttons-1.patch5.14 KB

Added button--primary, button--small, and button--danger.

Kept the class as button--danger for now just to keep them all in the same place and consistent. The actual CSS wouldn't change that much if we wanted to make the link--danger class work with <button>s.

It seems like we need a button--primary-danger class? It would be nice to cover situations where the primary action is also a dangerous action. Confirming a deletion is a core example.

The iframes might be messing with hover effects. Also had to guess the font-size for the small buttons. Looks good though!

LewisNyman’s picture

LewisNyman
he/him
CreditAttribution: LewisNyman commented
Status: Active » Needs review
oresh’s picture

oresh CreditAttribution: oresh commented
Status: Needs review » Needs work

- this style doesn't affect anything, can be deleted

.button:disabled:hover, .button.is-disabled:hover {
box-shadow: 0 0 0;
}

- Danger buttons have .boxshadow .button:focus applied when focused, and have border radius, thought there should be none or different focus.

- If you press red danger button and start dragging it - you'll get grey background gradient under it.

- At line 70 there is a class dublication:

.button:hover {
  box-shadow: 0 1px 2px hsla(0, 0%, 0%, 0.2);
}
.button:hover {
  box-shadow: 0 1px 2px hsla(0, 0%, 0%, 0.2);
}

Looks good on small screens, i think the font size for small buttons is ok.
Tested on Chrome v.24 on Linux

LewisNyman’s picture

LewisNyman
he/him
CreditAttribution: LewisNyman commented
Status: Needs work » Needs review
FileSize
buttons-4.patch4.73 KB

Fairly simple fixes, I've remove the button class from button--danger. That inheritance makes no sense.

oresh’s picture

oresh CreditAttribution: oresh commented

Awesome!
Only one more point, i think the box shadow transition looks really good on grey button, but on the blue button you get a strange two-step effect on hover.

The other thing - the bottom border is really to dark on hover, and the shadow visually increases the height of the button for a pixel or two (depending on screen brightness). And the shadow also makes it's deal with this dark border (1 extra dark pixel)

So when i hover the button, it looks like it's growing for 2 pixels, which is kind of weird. Lets do something like this:

.button--primary:hover {
box-shadow: 0 1px 2px hsla(0, 0%, 0%, 0.3);
border-bottom-color: #1669ad;
}

and remove transition for box-shadow.
Thank you!

ry5n’s picture

ry5n CreditAttribution: ry5n commented
FileSize
buttons-p4-with-p5.interdiff.txt8.24 KB
buttons-p5.txt6.69 KB

@oresh You’re right that the shadow transition was a bit funky. I think I’ve cleaned it up a bit in the attached patch. I also notice that I didn’t get the contrast quite right in Photoshop for the primary buttons (for the hover and active states) so I tweaked those a bit. I also added a bit more side padding to all the large buttons. Small buttons have a bit bigger text to match the design.

I also moved the danger classes into a separate set of styles, .link (to style non-anchors as links, for e.g. buttons) and .link--danger for the danger variant. I want there to be a clear-cut rule for variants that the base class must always appear with the variant class, so there’s no question that if I apply `.button--small` I *must* apply `.button` as well. This makes sense, since a variant class assumes the base styles and modifies them in some way. Does that make sense?

ry5n’s picture

ry5n CreditAttribution: ry5n commented

I’ve pushed the latest to the `buttons` branch. Live preview: http://drupalcode.org/sandbox/ry5n/1932040.git/blob_plain/refs/heads/but...

Bojhan’s picture

Bojhan CreditAttribution: Bojhan commented

The first thing I notice, besides them being quite cool :) Is that the blue button could use a tiny gradient. It looks a little flat compared to the other buttons.

@r5yn How do you feel about this?

ry5n’s picture

ry5n CreditAttribution: ry5n commented

@Bojhan I agree they should match a bit better. The Primary buttons actually have a gradient, but it’s subtle, and less noticeable than the standard buttons. I think because the Primary version is such a dark colour. I’ll be able to tackle this and some more issues in a few days.

Bojhan’s picture

Bojhan CreditAttribution: Bojhan commented

It's probably a little too subtle.

ry5n’s picture

ry5n CreditAttribution: ry5n commented
Status: Needs review » Reviewed & tested by the community

I think the gradient subtleties are something for later. For now, this is in good enough shape for an initial merge with master. Using RTBC as a shorthand for ‘ready for first merge’.