Refactor and introduce generic button CSS styles for forms and links

FWIW, my pedantic self also vastly prefers .button.

I only raised this, because I've seen so many instances of .btn in the meantime that I thought it would be worth to discuss.

I also just looked up what jQuery UI is doing for buttons, but it essentially does nothing, or rather, only applies a flood of custom, jUI-specific CSS classes via JavaScript:

ui-button ui-widget ui-state-default ui-corner-all ui-button-text-only

I'm fairly sure we don't want to go there. ;-)

Updated issue summary.

Yep, let's not get into scope-creep here though ;)

This issue should focus on .button and .button only. That said, looks like we have a winner. ;)

That essentially means we have a decision and can move forward with an actual patch. We're sorta blocked on #1238484: Ability to mark the form buttons to be of a specific type (so that they can be styled differently) though, since that one changes a dozen of form button styles across the board, so we should wait until that has been committed. :)

Not sure whether I covered everything from the summary, but attached is my initial prototype that I want to propose.

Looks + works fine in Seven. (fun, fun, we do not actually expect differences here ;))

Hah, I forgot about the fun part — merging the new a.button.add styles of action links :)

That said, what do we do about that .add class? It appears a bit out of line with the new .button-primary and .button-danger classes, no?

Do we want to rename that into .button-add?

Done so in attached patch.

+1,000 to #9 — I actually discussed that with many others in the past already, but never got around to create a dedicated issue for it. :)

However, let's do that in a separate follow-up issue. Since that issue will be bound to feature freeze, we have some pressure to get both this and that follow-up issue ready in time... ;)

Oh, and besides that, I additionally think that the action link buttons actually present a special kind of button with special visual importance and implications.

The only argumentation I'd be able to follow would be that the wrapping .action-links container provides sufficient context to style them differently... but then again, I'd think I'd like to see "add" buttons to appear in a consistent way throughout a site, regardless of whether they appear in the 'dedicated' action link buttons area/container or not. (?)

Isn't that the same as .button-add, just colored .action though? ;)

I'm open to whichever solution we can agree with :)

Two thoughts just crossed my mind:

1) Aren't all buttons actions? (from an interaction perspective) — We'd probably need a better name than .action, I think.

2) We're probably struggling hard, because the usability/visual/interface pattern for action links is not really clearly defined. In Seven, the links were presented as "Add something" operation and that's why we improved their styling and added the .button.add class recently — but reality is, not all action links that appear there are actually "Add something" links; a couple of pages in core are showing links that are rather of the form "Manage something" there. — In turn, .button-add essentially makes a big assumption which is not necessarily true, but the problem is not that it is making that assumption, but instead, the problem is that the pattern is not clearly defined and not consistently implemented.

3) .button-add would indeed presume that add-action-links ought to be styled as buttons. I can see the problem there. But at the same time, I also think that the application needs to set some basic interface pattern standards, in the same way we have tabs/local tasks and vertical tabs. In turn, I'm not sure whether making the assumption that action links should appear as buttons is good or bad (as in, too much).

Oyoyoy... my fear this is getting very meta ;)

Technically, this issue shouldn't really have to dive this deep into interface/pattern questions, since the main objective is a bare and simple refactoring of CSS classes. I wonder whether we can agree on a lowest common denominator and rather try to quickly move forward? :)

Would it make sense to create a follow-up issue for the .button-add vs. .action class name, so we can discuss that in isolation without blocking this issue? :)

OK, not sure I want to change the .action-links class as part of this issue, but otherwise, I think that makes sense.

Let me try to summarize:

1) We have .button + button for general button styling.

2) .button.add is changed into .button-action.

3) In a separate follow-up step, we move the visual "add icon" presentation into a new .icon-add class, which gets applied to action links.

Makes sense?

Attached patch implements that.

I do not see a visual change compared to HEAD in any core theme, so I'm not creating/attaching screenshots.

@ry5n: Cool with me, though that's actually a detail left for the icon follow-up issue for #9. ;) Also, please note that we have to create + discuss + implement + test the icon feature in no more than 7 days... so we better hurry up and get this partial blocker done ASAP. ;) I'll try to create the icon issue tonight.

Does that mean that we're actually RTBC here? :)

Reverted EntityFormController changes.

Those were irrelevant here and caused the test failures. We badly need to clean that up, but let's do that in a separate issue.

#23: button.23.patch queued for re-testing.

An interdiff would have been nice :)

(btw, I'm developing almost all of these patches in the Platform sandbox repository, and just granted you write access, so if you're versed enough with git and want to put your branches in there, go ahead :) The project page contains some instructions for branch names, and the git diffup alias performs all the other magic)

Yeah, I looked up the spec for the disabled attribute, too — but regardless of whether it applies to form controls only, is there a reason to qualify it with input? I didn't see one, so I left it unqualified. That said, I don't really care, too ;)

That said, the (pre-existing) .button styles in system.theme.css actually have unintended consequences:

Thus, I think we need to change those to .button-action, since that was their actual/original target/purpose.

What's also apparent by the screenshot is that we do not have any sensible default margin between buttons right now. This problem actually seems to be resolved independently by each theme (and thus, the solution is identical to all themes and should be resolved by system.theme.css instead).

Additionally done that, since it means a vast simplification across the board.

I also just had the neat :first-child margin-nuking idea for the button styles - which works for both LTR and RTL at the same time! :)

The approach works excellently for .button, because they are normally stacked up directly within a .form-actions container. It does not work for action link buttons, since each button is the only element within a list item of the action links item list. We continue to use the LTR/RTL margin-futzing for those list items.

That said, verifying and testing RTL revealed that the styles for action links are terribly broken currently ;) But luckily, the solution is to simply remove styles from Seven style-rtl.css — which is further, ultimate proof that we're on to some awesome design/theme experience in D8! :-)

(btw, I can't wait for design_test.module to get committed as part of the details patch, so we can simplify all of this horrible manual testing...)

No worries :)

To me this looks pretty much complete — let's make sure to quickly move forward here.

Adding a new issue tag :)

Meanwhile, also created: #1849712: Add theming template and prepare logic for rendering icons

That sounded very scary at first sight...

...but actually, I guess it's not only resolving that Toolbar case, but also the sorta upcoming case of #1719640: Use 'button' element instead of empty links — which intends to use button.link to essentially to do the opposite; i.e., styling buttons as links.

It also means we can revert the adjustments for .dropbutton button styles. (yay?)

In light of that, that makes perfect sense to me. It also seems to be in line with framework themes, which equally require you to apply the .button class to all elements.

Attached patch does so.

Yay, thanks! I agree, this looks ready to me.

Updated issue summary.

Updated issue summary accordingly.

This is plain markup/CSS class refactoring - screenshots do not show a difference, except for the now default/standardized horizontal margin between buttons in form actions.

Before:

After:

#34: core--refactor-button-css-1845728--34.patch queued for re-testing.

#34: core--refactor-button-css-1845728--34.patch queued for re-testing.

Re-rolled against HEAD.

#43: theme.button.43.patch queued for re-testing.