Problem/Motivation

There are three types of buttons supported in core and in the Seven style guide. Standard, primary, and danger.
It's important that the correct button types are used consistently throughout Drupal

Remaining tasks

  1. Write UI documentation on when and why to use each button type. Drupal's UI standards page would work for now until #2102191: Discuss the availiable solutions to document the Seven style guide is resolved.
  2. Audit all forms in core, building a list of incorrect button types.
  3. Fix them

Test pages

  • Views modal forms. To reproduce: admin/structure/views/view/content > edit any field
  • Delete a view form. To reproduce: admin/structure/views/view/content/delete. I would suggest to use the "danger" button for the "Delete"/submit action to avoid accidentally deleted views.
  • Edit image styles effects form. To reproduce: admin/config/media/image-styles/manage/xx/add/image_crop.
  • Manage permissions form. To reproduce: admin/people/permissions.
  • Install new theme form / update form. To reproduce: admin/theme/install
  • Extend/modules page. To reproduce: admin/modules
CommentFileSizeAuthor
#34 form-failure-after-hitting-cancel.png178.69 KBtlilleberg
#34 2166473-correct-button-types-33.patch4.07 KBtlilleberg
#32 2166473-correct-button-types-32.patch4.07 KBcachee
#31 2166473-correct-button-types-31.patch4.1 KBcachee
#30 2166473-correct-button-types-30.patch4.1 KBdaviddemello
#29 install-theme-button-d8.png174.86 KBtlilleberg
#28 pic3.png2.98 KBcachee
#27 pic5.png8.69 KBcachee
#27 pic4.png2.18 KBcachee
#27 pic3.png2.18 KBcachee
#27 pic2.png4.56 KBcachee
#27 pic1.png4.63 KBcachee
#23 2166473-correct-button-types-22.patch3.45 KBlauriii
#16 2166473-correct-button-types-16.patch3.45 KBckrina
#14 2166473-correct-button-types-14.patch4.01 KBckrina
#6 2166473-compare-screenshots-6.png171.97 KBckrina
#6 2166473-correct-button-types-6.patch4.2 KBckrina
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

LewisNyman’s picture

LewisNyman
he/him
CreditAttribution: LewisNyman commented

I've updated the guidance on button in the UI guidelines: https://drupal.org/node/1850248

LewisNyman’s picture

LewisNyman
he/him
CreditAttribution: LewisNyman commented
Issue summary: View changes
ckrina’s picture

ckrina
she/her
Primary language Catalan
Location Barcelona
CreditAttribution: ckrina commented
Assigned: Unassigned » ckrina

Working on that.

ckrina’s picture

ckrina
she/her
Primary language Catalan
Location Barcelona
CreditAttribution: ckrina commented

To change:

  • Views modal forms. To reproduce: admin/structure/views/view/content > edit any field > The "remove" button should be a "danger" button.
  • Delete a view form. To reproduce: admin/structure/views/view/content/delete. The "cancel" button should be the "standard" one. Además, the "danger" button in this form is also the primary one, the form submit button. Anyway, I would suggest to use the "danger" button for that to avoid accidentally deleted views.
  • Edit image styles effects form. To reproduce: admin/config/media/image-styles/manage/xx/add/image_crop. "Cancel" button should be changed to "standard".

Not sure, to discuss:

  • Manage permissions form. To reproduce: admin/people/permissions. "Save permissions" button should be the "primary" one?
  • Install new theme form. To reproduce: admin/theme/install. The submit button should be the "primay"?
  • Edit menu form. To reproduce: admin/structure/menu/manage/account. There are 2 primary buttons: "Add link" and "Save". The last one is for the draggable table for links, the form's submit button. I would suggest to follow the way it's used on admin/modules (first "primary", second just "standard").
  • Block layout page. To reproduce: admin/structure/menu/manage/account. There are 2 primary buttons: "Add custom block" and "Save blocks". The last one is for the draggable table, the submit button of the form. I would suggest to follow the way it's used on admin/modules (first "primary", second just "standard").
  • Text formats and editors page. To reproduce: admin/config/content/formats. There are 2 primary buttons: "Add text format" and "Save changes". The last one is for the draggable table, the submit button of the form. I would suggest to follow the way it's used on admin/modules (first "primary", second just "standard").
  • Text formats and editors page. To reproduce: admin/config/user-interface/shortcut/manage/default/customize There are 2 primary buttons: "Add shortcut" and "Save changes". The last one is for the draggable table, the submit button of the form. I would suggest to follow the way it's used on admin/modules (first "primary", second just "standard").
LewisNyman’s picture

LewisNyman
he/him
CreditAttribution: LewisNyman commented

We discussed this in person but current there are situations where we have action links that look like primary buttons but we also need a primary button for the main form on the page. This is a bit of weird use case at the moment.

ckrina’s picture

ckrina
she/her
Primary language Catalan
Location Barcelona
CreditAttribution: ckrina commented
FileSize
2166473-correct-button-types-6.patch4.2 KB
2166473-compare-screenshots-6.png171.97 KB

Here is the patch for the following forms:

  • Views modal forms. To reproduce: admin/structure/views/view/content > edit any field
  • Delete a view form. To reproduce: admin/structure/views/view/content/delete. I would suggest to use the "danger" button for the "Delete"/submit action to avoid accidentally deleted views.
  • Edit image styles effects form. To reproduce: admin/config/media/image-styles/manage/xx/add/image_crop.
  • Manage permissions form. To reproduce: admin/people/permissions.
  • Install new theme form / update form. To reproduce: admin/theme/install
  • Extend/modules page. To reproduce: admin/modules

ckrina’s picture

ckrina
she/her
Primary language Catalan
Location Barcelona
CreditAttribution: ckrina commented
Status: Active » Needs review

Updating status.

Status: Needs review » Needs work

The last submitted patch, 6: 2166473-correct-button-types-6.patch, failed testing.

LewisNyman’s picture

LewisNyman
he/him
CreditAttribution: LewisNyman commented

6: 2166473-correct-button-types-6.patch queued for re-testing.

LewisNyman’s picture

LewisNyman
he/him
CreditAttribution: LewisNyman commented
Related issues: +#2229157: Add correct button types to account settings forms

Love this patch! It's so nice to fix these pages. Hopefully it will pass this time.

While clicking around I found some more pages. Let's try and keep these patches small #2229157: Add correct button types to account settings forms

The last submitted patch, 6: 2166473-correct-button-types-6.patch, failed testing.

ckrina’s picture

ckrina
she/her
Primary language Catalan
Location Barcelona
CreditAttribution: ckrina commented
Assigned: ckrina » Unassigned

I'm sorry, but I do not have time to work on it. I hope someone can take it over. Unasigning.

ckrina’s picture

ckrina
she/her
Primary language Catalan
Location Barcelona
CreditAttribution: ckrina commented
Assigned: Unassigned » ckrina
Issue tags: +DrupalCampSpain

Rerolled patch.

ckrina’s picture

ckrina
she/her
Primary language Catalan
Location Barcelona
CreditAttribution: ckrina commented
Status: Needs work » Needs review
FileSize
2166473-correct-button-types-14.patch4.01 KB

Here the file!

Status: Needs review » Needs work

The last submitted patch, 14: 2166473-correct-button-types-14.patch, failed testing.

ckrina’s picture

ckrina
she/her
Primary language Catalan
Location Barcelona
CreditAttribution: ckrina commented
FileSize
2166473-correct-button-types-16.patch3.45 KB

Trying again.

ckrina’s picture

ckrina
she/her
Primary language Catalan
Location Barcelona
CreditAttribution: ckrina commented
Status: Needs work » Needs review
LewisNyman’s picture

LewisNyman
he/him
CreditAttribution: LewisNyman commented
Status: Needs review » Needs work 
+++ b/core/modules/views_ui/lib/Drupal/views_ui/ViewDeleteForm.php
@@ -33,8 +33,20 @@ public function getCancelRoute() {
-  public function getConfirmText() {
-    return $this->t('Delete');
+  public function buildForm(array $form, array &$form_state) {
+    $form = parent::buildForm($form, $form_state);
+
+    $form['actions'] = array('#type' => 'actions');
+    $form['actions']['submit'] = array(
+      '#type' => 'submit',
+      '#button_type' => 'primary',
+      '#value' => $this->t('Delete')
...
   /**

Hey, I tried this out and the delete confirmation form now seems to stay on an endless loop?

The last submitted patch, 16: 2166473-correct-button-types-16.patch, failed testing.

LewisNyman’s picture

LewisNyman
he/him
CreditAttribution: LewisNyman commented
Issue tags: +frontend
ckrina’s picture

ckrina
she/her
Primary language Catalan
Location Barcelona
CreditAttribution: ckrina commented
Assigned: ckrina » Unassigned

Unassigning that one to let someone with time to work on that.

lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
CreditAttribution: lauriii commented
Assigned: Unassigned » lauriii
lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
CreditAttribution: lauriii commented
Assigned: lauriii » Unassigned
Status: Needs work » Needs review
FileSize
2166473-correct-button-types-22.patch3.45 KB

I rerolled patch to PSR-4 and fixed failing tests.

LewisNyman’s picture

LewisNyman
he/him
CreditAttribution: LewisNyman commented
Issue summary: View changes
Issue tags: -DrupalCampSpain +Needs manual testing, +Novice

I've updated the issue summary with the test pages.

daviddemello’s picture

daviddemello CreditAttribution: daviddemello commented
Assigned: Unassigned » daviddemello
daviddemello’s picture

daviddemello CreditAttribution: daviddemello commented
FileSize
button-type-screenshots-before-after.zip152.32 KB

Images of before/after states are attached.
The case "Install new theme form / update form. To reproduce: admin/theme/install" is throwing an error at the moment, so no after picture is possible.

cachee’s picture

cachee CreditAttribution: cachee commented
FileSize
pic1.png4.63 KB
pic2.png4.56 KB
pic3.png2.18 KB
pic4.png2.18 KB
pic5.png8.69 KB

Test steps:
1. Downloaded the latest version of D8
2. Verified the buttons were incorrect for the following:
• Views modal forms. To reproduce: admin/structure/views/view/content > edit any field
• Delete a view form. To reproduce: admin/structure/views/view/content/delete. I would suggest to use the "danger" button for the "Delete"/submit action to avoid accidentally deleted views.
• Edit image styles effects form. To reproduce: admin/config/media/image-styles/manage/xx/add/image_crop.
• Manage permissions form. To reproduce: admin/people/permissions.
• Install new theme form / update form. To reproduce: admin/theme/install (could not access the link to load modules/themes from the drupal install)
• Extend/modules page. To reproduce: admin/modules
3. Performed the following tests:
• Created a views and field: the buttons were good
Image 1
• Managed permission button was good
Pic 2
• Extend a module: button was good
Pic 3
4. The following button did not work:
• Update image style option button css missing
Pic 4
• Could not access the admin/theme/install page due to a core issue InvalidArgumentException: No check has been registered for access_check.update.manager_access in Drupal\Core\Access\AccessManager->loadCheck() (line 384 of C:\Users\Caroline\Sites\UniServer\www\d8\core\lib\Drupal\Core\Access\AccessManager.php).

Pic 5

cachee’s picture

cachee CreditAttribution: cachee commented
FileSize
pic3.png2.98 KB

Test steps:
1. Downloaded the latest version of D8
2. Verified the buttons were incorrect for the following:
• Views modal forms. To reproduce: admin/structure/views/view/content > edit any field
• Delete a view form. To reproduce: admin/structure/views/view/content/delete. I would suggest to use the "danger" button for the "Delete"/submit action to avoid accidentally deleted views.
• Edit image styles effects form. To reproduce: admin/config/media/image-styles/manage/xx/add/image_crop.
• Manage permissions form. To reproduce: admin/people/permissions.
• Install new theme form / update form. To reproduce: admin/theme/install (could not access the link to load modules/themes from the drupal install)
• Extend/modules page. To reproduce: admin/modules
3. Performed the following tests:
• Created a views and field: the buttons were good
Image 1
• Managed permission button was good
Pic 2
• Extend a module: button was good
Pic 3
4. The following button did not work:
• Update image style option button css missing
Pic 5
• Could not access the admin/theme/install page due to a core issue InvalidArgumentException: No check has been registered for access_check.update.manager_access in Drupal\Core\Access\AccessManager->loadCheck() (line 384 of C:\Users\Caroline\Sites\UniServer\www\d8\core\lib\Drupal\Core\Access\AccessManager.php).

tlilleberg’s picture

tlilleberg CreditAttribution: tlilleberg commented
FileSize
install-theme-button-d8.png174.86 KB

With the latest version of d8, I can see admin/theme/install and can confirm the button was not primary before, and the patch fixes it. I don't know if it's the problem, but if you don't have the updates modules enabled you can't access the theme/install page.

The issue with admin/config/media/image-styles/manage/xx/add/image_crop is that "cancel" is a link, not a submit button in the render array, so it doesn't take the css. I tried turning it into a submit button, but then if you try to add an image-style and then cancel out, the "required" html5 attributes on 'width' and 'height' will not validate and will not let you cancel.

Some options regarding the image-styles form are:

  1. Altering the css on those particular cancel links to look like the grey buttons.
  2. Using ->validateForm() instead of HTML required attributes to validate the form.

-- Travis Lilleberg
Digital Loom
www.digital-loom.com

daviddemello’s picture

daviddemello CreditAttribution: daviddemello commented
FileSize
2166473-correct-button-types-30.patch4.1 KB

Edited buildForm() in /core/modules/image/src/Form/ImageEffectAddForm - overriding the base class ImageEffectFormBase::ImageEffectFormBase() to output the cancel action as a submit button instead of as a link.

Thought that in the future it might be good to either change the cancel action in the base class for consistency throughout Drupal, or to provide and affordance (argument or helper function) to set the behavior of the cancel action - avoiding knowledge and repetition of the implementation when sublassing buildForm() in other places.

cachee’s picture

cachee CreditAttribution: cachee commented
FileSize
2166473-correct-button-types-31.patch4.1 KB

Added all of the button attributes to the image cancel button in the /core/modules/image/src/Form/ImageEffectAddForm.

cachee’s picture

cachee CreditAttribution: cachee commented
FileSize
2166473-correct-button-types-32.patch4.07 KB

Changed the button code to not be a primary button.
Only local images are allowed.

lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
CreditAttribution: lauriii commented
Status: Needs review » Needs work

Looks good!

+++ b/core/modules/user/src/Form/UserPermissionsForm.php
@@ -172,7 +172,10 @@ public function buildForm(array $form, array &$form_state) {
+      '#value' => $this->t('Save permissions'));

Closing should be on the next line. More about this here.

tlilleberg’s picture

tlilleberg CreditAttribution: tlilleberg commented
FileSize
2166473-correct-button-types-33.patch4.07 KB
form-failure-after-hitting-cancel.png178.69 KB

There's an issue with patches 31/32 in the ImageEffectsFormBase class.

To recreate:

  1. Apply the 31/32 patch
  2. Go to admin/config/media/image-styles/manage/large
  3. Add a new crop effect by selecting 'crop' from the select list and hitting "Add"
  4. Now try to cancel out

It won't let you cancel because we've made the cancel link into a submit button and because of that, it tries to validate the form. The empty width/height prevent you from leaving.

I address this by keeping "cancel" a link and just adding the button class to it.

I also fixed the coding standard issue lauriii saw in the User Permission Form

-- Travis Lilleberg
Digital Loom
www.digital-loom.com

LewisNyman’s picture

LewisNyman
he/him
CreditAttribution: LewisNyman commented

It seems dangerous to change the button too much. if they aren't submit button we could just add the classes instead of using #button_type?

lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
CreditAttribution: lauriii commented
Issue tags: +Needs tests

If there's no tests for this, we should extend the tests to cover this.

tlilleberg’s picture

tlilleberg CreditAttribution: tlilleberg commented

Just to be clear, with patch 33, the cancel button is *not* a submit button. I did what Lewis suggested and left it a link and simply added the 'button' class to it. That was the only cancel button we created. All the rest of the altered buttons should be submit actions.

So @Lewis, patch 33 should do what you suggested, but I can definitely update it if you find something else that needs fixing.

I can work on the test if nobody beats me to it, but that'll take a little longer.

LewisNyman’s picture

LewisNyman
he/him
CreditAttribution: LewisNyman commented
Issue tags: -styleguide-conform, -Novice

@tlilleberg Yes, I think the problem is in the views delete form?

LewisNyman’s picture

LewisNyman
he/him
CreditAttribution: LewisNyman commented
Assigned: daviddemello » Unassigned
LewisNyman’s picture

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

LewisNyman
he/him
CreditAttribution: LewisNyman commented

This issue has stalled because of one difficult problem on one page. I think the best thing to do here is to split this up into separate issues for each page, these can be nice novice tasks for Dev days

LewisNyman’s picture

LewisNyman
he/him
CreditAttribution: LewisNyman commented
Status: Needs review » Active

I'm setting this issue to active as it is now a meta issue without the need for any patches. All the patches can be submitted and discussed in the individual issues:

#2469889: The modules page doesn't have a primary button
#2469911: The edit image styles form's cancel link should be styled a a button
#2469917: The install new theme page doesn't have a primary button
#2469921: The appearance page doesn't have a primary button
#2469929: The confirmation forms' cancel link should be styled as a button
#2469939: The permissions page doesn't have a primary button

joelpittet’s picture

joelpittet
Primary language English
Location Vancouver
CreditAttribution: joelpittet commented
Category: Task » Plan
pingwin4eg’s picture

pingwin4eg
Location Zaporizhia 🇺🇦
CreditAttribution: pingwin4eg as a volunteer commented

There's another one issue: #2630162: The Edit URL Alias form buttons should be styled in consistency with other buttons

Version: 8.0.x-dev » 8.1.x-dev

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

Bug reports should be targeted against the 8.1.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

Bug reports should be targeted against the 8.2.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.6 was released on February 1, 2017 and is the final full bugfix release for the Drupal 8.2.x series. Drupal 8.2.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.3.0 on April 5, 2017. (Drupal 8.3.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.3.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.6 was released on August 2, 2017 and is the final full bugfix release for the Drupal 8.3.x series. Drupal 8.3.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.4.0 on October 4, 2017. (Drupal 8.4.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.4.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.4 was released on January 3, 2018 and is the final full bugfix release for the Drupal 8.4.x series. Drupal 8.4.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.5.0 on March 7, 2018. (Drupal 8.5.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.5.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.6 was released on August 1, 2018 and is the final bugfix release for the Drupal 8.5.x series. Drupal 8.5.x will not receive any further development aside from security fixes. Sites should prepare to update to 8.6.0 on September 5, 2018. (Drupal 8.6.0-rc1 is available for testing.)

Bug reports should be targeted against the 8.6.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

pingwin4eg’s picture

pingwin4eg
Location Zaporizhia 🇺🇦
CreditAttribution: pingwin4eg as a volunteer commented
Status: Active » Fixed

All child issues had been fixed a long time ago.

Status: Fixed » Closed (fixed)

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