Rewrite button components inline with our CSS standards

Reviewed the code on both css files. Looked okay.

I merged the css styles.

button--danger looks a bit misplaced after the link style. Maybe should be moved to next to the modifier "primary" ?

Best regards

Sorry new path there was a indent problem with one of the styles in the previous one.

@joaogarin Don't forget to set the issue status to 'Needs review' when you upload a patch :)

Hello,

Here goes a new patch. removed the unwanted file and moved the action links comment in with the rest of the description.

Also attaching some print screens of the buttons.

1. The buttons.theme.css file is deleted, but it is still referenced in seven.libraries.yml

2. /**
    * Rare instances when a dropbutton is actually just a button.
    * Copied from Seven's buttons.theme.css.
    */

   There is a reference to buttons.theme.css in dropbutton.component.css that should be updated.

3. +++ b/core/themes/seven/css/components/buttons.css
   @@ -25,15 +25,123 @@
   +  -moz-appearance: none;  /* " */
   +  padding: 4px 1.5em;  /* 1 */
   ...
   +  font-size: 0.875rem;  /* 2 */
   ...
   +  -webkit-font-smoothing: antialiased;  /* 3 */
   

   These numbered comments should be incremented and the original comments should be added to the selector docblock.

4. +++ b/core/themes/seven/css/components/buttons.css
   @@ -25,15 +25,123 @@
   +  color: #333333;
   

   Write shorthand color codes when possible, eg. #333 instead of #333.

Hello,

Did the revisions by @idebr :

Removed reference from seven.libraries.yml
Changed reference in dropbutton.component.css (now referencing buttons.css)
Reworked the comments in the buttons.css stylesheet

I am adding also some screenshots.

Hello!

I reviewed the patch in #13, and I have rerolled with these minor changes:

1. changed the #333333 color to the shorthand

-  color: #333333;
+  color: #333;

2. Removed the indentation for the -moz- vendor properties, because I could not find indication that we want this indentation in the css guidelines (correct me if I'm wrong!) :)

   -webkit-appearance: none;
-     -moz-appearance: none;
+  -moz-appearance: none;

3. Changed the second comment for comment number 3, having the number there instead of the / " / character (seems clearer that way, I think)

-webkit-appearance: none;  /* 3 */
-     -moz-appearance: none;  /* " */
+  -moz-appearance: none;  /* 3 */

I don't think there are any references regarding the comments or the indentation so I decided to leave them like they where.

I actually think the comment /* '' */ makes sense as this is a universal symbol for referencing what is on top, so in that sense the indentation would make sense because if references what is exactly above

http://en.wikipedia.org/wiki/Ditto_mark

Anyway as there is no reference in the guides I think we can agree on a good solution and go with it;)

Ok, I understand the reason now!

My opinion on this would be to go with removing the indentation however, because of what this says:

https://www.drupal.org/node/1887862#whitespace

We need a third opinion probably :)

Yes looking at https://www.drupal.org/node/1887862#blank-lines it actually has the indentation on :

* A comment describing the ruleset. */
.selector-1,
.selector-2,
.selector-3[type="text"] {
-webkit-box-sizing: border-box;
-moz-box-sizing: border-box;

Altough its talking about the blank lines and not indentation I think we can see from the example that is using it. Anyway yes lets have someone else give their opinions;)

Hello, how do we proceed with this? Proceed from #11 or #13?

i can incorporate the change for the minified color #333 into #11 and put the new patch.

The patch no longer applies:

error: patch failed: core/themes/seven/seven.libraries.yml:12
error: core/themes/seven/seven.libraries.yml: patch does not apply

Regarding the white-space: there is no explicit rule regarding the use of white-space in vendor prefixes that I am aware of, but currently in HEAD the version without additional white-space is more common so let's stick with that for consistency.

Here's a straight reroll of #13.

I'm going to agree with @vermario here that using the number instead of the " ensures no one will be confused, and removing the indentation looks better, although I don't think either change is necessarily dictated by the coding standard.

Went through the code standards. No indents anywhere for vendor prefixes. Found two more in the file and made patch to work with those as well. Applies well to latest Dev.

Interdiff in a weird format but you can see the changes easily still. The diff is very minimal from the last one, and could mark as RTBC really quickly.

Looks good, however I made one more small coding standards fix, to the spacing of the wrapped line in the comment.

CSS is not frozen in beta. Committed 262a3e7 and pushed to 8.0.x. Thanks!