Use the small button component for Claro:

Buttons should gain the new buton--small class for Claro to use the smaller variant of buttons in the UI.

.button--small

should be added to the markup of the "Remove" & "Add" buttons.

CommentFileSizeAuthor
#10 claro-small-buttons-3111075-10-interdiff.txt877 bytesgaards
#10 claro-small-buttons-3111075-10.patch5.03 KBgaards
#7 claro-small-buttons-3111075-7-interdiff.txt3.58 KBsasanikolic
#7 claro-small-buttons-3111075-7.patch4.18 KBsasanikolic
#7 Button bugs.png100.18 KBsasanikolic
#7 Dropdown paddings after.png70.98 KBsasanikolic
#7 Dropdown paddings before.png49.22 KBsasanikolic
#3 claro-small-buttons-3111075-3.patch1.68 KBsaschaeggi

Comments

saschaeggi created an issue. See original summary.

saschaeggi’s picture

saschaeggi
he/him
commented
Title: Use small "Remove" button component in Claro » Use small button component for Claro
Issue summary: View changes
saschaeggi’s picture

saschaeggi
he/him
commented
StatusFileSize
new claro-small-buttons-3111075-3.patch1.68 KB

Patch for small "Add" & "Remove" buttons.

saschaeggi’s picture

saschaeggi
he/him
commented
Status: Active » Needs review

Status: Needs review » Needs work

The last submitted patch, 3: claro-small-buttons-3111075-3.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

saschaeggi’s picture

saschaeggi
he/him
commented
Status: Needs work » Needs review
sasanikolic’s picture

sasanikolic commented
StatusFileSize
new Dropdown paddings before.png49.22 KB
new Dropdown paddings after.png70.98 KB
new Button bugs.png100.18 KB
new claro-small-buttons-3111075-7.patch4.18 KB
new claro-small-buttons-3111075-7-interdiff.txt3.58 KB

I noticed some more bigger buttons, especially fixed the ones in the dropdown coming from paragraphs_library. Also fixed some margins there.

The "Add Paragraph" button was left big on purpose after discussing it with @berdir, since it's the primary button and should be bigger, IMHO.
Maybe we need some maintainers input regarding those decisions (@miro_dietiker) before continuing here.

Also noticed various issues when you change the add mode to just "Buttons", for example, paddings are missing below buttons. And nested "Add paragraph" buttons are missing right margin between the text and the button. Will create a followup for that.

Status: Needs review » Needs work

The last submitted patch, 7: claro-small-buttons-3111075-7.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

berdir’s picture

berdir
Primary language German
Location Switzerland
commented

Miro confirmed that's OK, but looks like we have a test fail that needs to be fixed here?

gaards’s picture

gaards commented
Status: Needs work » Needs review
StatusFileSize
new claro-small-buttons-3111075-10.patch5.03 KB
new claro-small-buttons-3111075-10-interdiff.txt877 bytes

I added a patch that should fix the test.

saschaeggi’s picture

saschaeggi
he/him
commented
Status: Needs review » Reviewed & tested by the community

@berdir can you have another look at it with the patch from gaards? It works for me and it passes :) Thanks!

  • Berdir committed 4b79026 on 8.x-1.x authored by gaards 
    Issue #3111075 by sasanikolic, gaards, saschaeggi: Use small button...
berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Status: Reviewed & tested by the community » Fixed

Seems like we shouldn't hardcode the exact list of classes and their order in the test, but this isn't making it worse. So, lets do this.

Status: Fixed » Closed (fixed)

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