Problem/Motivation

View UI edit form provides smaller Dropbutton by providing extra CSS in its admin styling.

The problem is that it is impossible to re-use the smaller variant of Views UI Dropbutton without repeating the CSS.

Proposed resolution

Provide a way (by an optional property) to specify Dropbutton variants (e.g. in the same way that Button variants provided).

Example:

$form['actions']['extra_actions'] = array(
  '#type' => 'dropbutton',
  '#dropbutton_type' => 'small',
  '#links' => array(
     'demo' => array(
       'title' => $this->t('Build Demo'),
      'url' => Url::fromRoute('fapi_example.build_demo'),
    ),
  ),
);

Remaining tasks

None.

User interface changes

None.

API changes

Optional new #dropbutton_type property on the dropbutton render element.

Data model changes

None.

Release notes snippet

None.

CommentFileSizeAuthor
#16 3057581-16.drupal.Add-support-for-Dropbutton-variants.patch1.29 KBfhaeberle
#16 interdiff.3057581.13-16.txt876 bytesfhaeberle
#13 3057581-add-support-dropbutton-variants-13.patch2.14 KBquiron
#5 3057581-add-support-dropbutton-variants-5.patch1.57 KBquiron
#3 3057581-add-support-dropbutton-variants-2.patch1.53 KBquiron
#2 3057581-add-support-dropbutton-variants.patch1.53 KBquiron

Comments

huzooka created an issue. See original summary.

quiron’s picture

quiron
Primary language Catalan
Location Barcelona
commented
StatusFileSize
new 3057581-add-support-dropbutton-variants.patch1.53 KB

This patch adds the same logic used for button variants to dropbutton

The css code applying the small style to the views_ui is:
https://git.drupalcode.org/project/drupal/blob/8.8.x/core/modules/views_...

quiron’s picture

quiron
Primary language Catalan
Location Barcelona
commented
StatusFileSize
new 3057581-add-support-dropbutton-variants-2.patch1.53 KB

Sorry, extra actions in views edit page should be small and not extrasmall.

lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
commented
Status: Active » Needs review
quiron’s picture

quiron
Primary language Catalan
Location Barcelona
commented
StatusFileSize
new 3057581-add-support-dropbutton-variants-5.patch1.57 KB

Fixed patch base directory.

fhaeberle’s picture

fhaeberle
he/him
Location 🇩🇪 Stuttgart
commented
Status: Needs review » Reviewed & tested by the community

Looks good from my point of view!

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 5: 3057581-add-support-dropbutton-variants-5.patch, failed testing. View results

fhaeberle’s picture

fhaeberle
he/him
Location 🇩🇪 Stuttgart
commented

Mmh, the failing test is strange. Can' see why the test in /core/tests/Drupal/FunctionalJavascriptTests/Ajax/DialogTest.php:61 fails.
Can somebody help?

yogeshmpawar’s picture

yogeshmpawar
He/Him/His
Primary language English
Location Pune(MH), INDIA 🇮🇳
commented
Status: Needs work » Reviewed & tested by the community

Above test fails are unrelated so setting back to RTBC.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 5: 3057581-add-support-dropbutton-variants-5.patch, failed testing. View results

bnjmnm’s picture

bnjmnm
he/him
Primary language English
Location Ann Arbor, MI
commented
Status: Needs work » Reviewed & tested by the community

Setting back to RTBC as this was kicked back to needs work due to a random fail in a completely unrelated test.

lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
commented
Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs change record

We should document this in the render element so that when people look at the documentation for dropbutton render element, they can actually see this configuration option.

We should also create a change record to notify people of this new feature.

quiron’s picture

quiron
Primary language Catalan
Location Barcelona
commented
Status: Needs work » Needs review
Issue tags: -Needs change record
StatusFileSize
new 3057581-add-support-dropbutton-variants-13.patch2.14 KB

Added the documentation in the comment.
Also created a draft change record for it https://www.drupal.org/node/3072313

fhaeberle’s picture

fhaeberle
he/him
Location 🇩🇪 Stuttgart
commented
Status: Needs review » Reviewed & tested by the community

Reviewed this and it looks good for me. Added a bit more details to the change record. RTBC.

lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
commented
Status: Reviewed & tested by the community » Needs work 
+++ b/core/lib/Drupal/Core/Render/Element/Dropbutton.php
--- a/core/modules/views_ui/src/ViewEditForm.php
+++ b/core/modules/views_ui/src/ViewEditForm.php

+++ b/core/modules/views_ui/src/ViewEditForm.php
@@ -702,6 +702,7 @@ public function renderDisplayTop(ViewUI $view) {
+      '#dropbutton_type' => 'small',

@@ -1013,6 +1014,7 @@ public function getFormBucket(ViewUI $view, $type, $display) {
+      '#dropbutton_type' => 'extrasmall',

I think we should remove these classes from this patch since these are specific to Claro. Claro would be still responsible for adding these, but it would be easier because of the added support to Dropbutton render element.

fhaeberle’s picture

fhaeberle
he/him
Location 🇩🇪 Stuttgart
commented
Status: Needs work » Needs review
StatusFileSize
new interdiff.3057581.13-16.txt876 bytes
new 3057581-16.drupal.Add-support-for-Dropbutton-variants.patch1.29 KB

Removed the classes but I'm not sure, what's the default dropbutton class? Or should it default to none?

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Status: Needs review » Reviewed & tested by the community

#15++ for not adding (contrib) theme-specific values to core code, and certainly to module code.

#16 Yes, it should default to none. Which is exactly what you did in #16 :)

The code in #16 now matches https://git.drupalcode.org/project/drupal/blob/8.8.x/core/lib/Drupal/Cor... exactly. This is now an easy RTBC: it's applying a pre-existing pattern from one render element to another render element 👍

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Issue summary: View changes
Issue tags: +Needs change record

Still needs change record and issue summary update for the new capability being added here.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Issue summary: View changes
Issue tags: -Needs change record

Oops, a change record already exists! 😅 👍

lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
commented
Issue summary: View changes
Status: Reviewed & tested by the community » Fixed

Committed e707588 and pushed to 8.8.x. Thanks!

  • lauriii committed e707588 on 8.8.x 
    Issue #3057581 by quiron, fhaeberle, Wim Leers, lauriii: Add support for...

Status: Fixed » Closed (fixed)

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