Claro theme loses actions for the view bulk operations form

Created on 16 May 2024, about 1 month ago
Updated 19 June 2024, 6 days ago

Problem/Motivation

The Claro theme provides form alter(claro_form_alter) to change the position for the action buttons to make possible a new design for the bulk operations views - https://www.drupal.org/project/drupal/issues/3070558 📌 Implement bulk operation designs Fixed

However, this form alteration provides an issue when the additional action buttons disappear from the form. The reason for this is the code unsets actions array without checking if it contains any other actions except the submit action - https://git.drupalcode.org/project/drupal/-/blob/11.x/core/themes/claro/...

Steps to reproduce

  1. Install modules views_bulk_operations and draggableviews.
  2. Configure a view that uses both modules.
  3. You will find that the view shows only the VBO operation, but hides the "Save order" button for the draggableviews module.

Proposed resolution

At the moment, I think that we can simply move buttons from the "actions" array into "bulk_actions_container".
However, it might require additional changes in styles.

🐛 Bug report
Status

RTBC

Version

11.0 🔥

Component
Claro 

Last updated about 5 hours ago

Created by

🇺🇦Ukraine _tarik_ Lutsk

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

  • Issue created by @_tarik_
  • Issue was unassigned.
  • Status changed to Needs review about 1 month ago
  • 🇺🇦Ukraine _tarik_ Lutsk

    I have added simple code that will move additional action buttons before the actions array are deleted from the form.

    FYI: The changes are compatible with the Drupal 10.2.5

  • Pipeline finished with Success
    about 1 month ago
    Total: 571s
    #174379
  • Status changed to Needs work about 1 month ago
  • 🇺🇸United States smustgrave

    Question is this reproducible in just core and only with the contrib?

    Believe we will need a test case to show the issue.

  • 🇺🇸United States bnjmnm Ann Arbor, MI

    This can be reproduced without contrib, but will require a simple test module that form_alters the views bulk operations form and adds an additional item to $form['actions']. In HEAD, you can confirm the reported bug is occuring as that new item will not appear in the views bulk form. The solution is for Claro to still unset $form["actions"]["submit"] but anything else in $form["actions"] should get moved to $form['bulk_actions_container']

  • First commit to issue fork.
  • Pipeline finished with Failed
    20 days ago
    Total: 180s
    #192420
  • Pipeline finished with Success
    20 days ago
    Total: 589s
    #192431
  • Status changed to Needs review 20 days ago
  • Added a test module that alters the form and also added test that verifies that the issue exists and also covering the solution.

  • Status changed to Needs work 19 days ago
  • 🇺🇸United States bnjmnm Ann Arbor, MI

    Can this get a test-only branch or patch to confirm it fails without the fix? Be sure to customize drupalci so it runs only this newly added test and not the full 30 minutes of Drupal core tests.

  • Pipeline finished with Success
    19 days ago
    Total: 566s
    #193307
  • Status changed to Needs review 19 days ago
  • Pipeline finished with Success
    19 days ago
    Total: 506s
    #193313
  • Status changed to Needs work 13 days ago
  • 🇺🇸United States smustgrave

    Left some comments

    But tests appear to be passing when assuming they should be failing correct/

  • But tests appear to be passing when assuming they should be failing correct/

    @smustgrave if you look at the tests,it asserts that the button is not present.As Ben mention in #9 🐛 Claro theme loses actions for the view bulk operations form Needs review that we only want a test to confirm that the issue exist but not the fix(alteast latest till #9).

  • Pipeline finished with Success
    13 days ago
    Total: 514s
    #197910
  • Status changed to Needs review 13 days ago
  • Utkarsh_33 changed the visibility of the branch 3447611-claro-theme-loses-test-only to hidden.

  • Utkarsh_33 changed the visibility of the branch 3447611-claro-theme-loses-test-only to active.

  • Pipeline finished with Success
    12 days ago
    Total: 621s
    #198058
  • Pipeline finished with Failed
    12 days ago
    Total: 1771s
    #198068
  • 🇺🇸United States smustgrave

    Right so I would of expected the test only branch to fail and show the bug

  • Pipeline finished with Failed
    12 days ago
    Total: 647s
    #198539
  • Pipeline finished with Success
    12 days ago
    Total: 604s
    #198567
  • 🇺🇸United States smustgrave

    smustgrave changed the visibility of the branch 3447611-claro-theme-loses-test-only to hidden.

  • Status changed to RTBC 6 days ago
  • 🇺🇸United States smustgrave
    1) Drupal\FunctionalJavascriptTests\Theme\ClaroViewsBulkOperationsTest::testViewBulkOperationAlter
    Behat\Mink\Exception\ElementNotFoundException: Button with id|name|label|value "Custom button" not found.
    /builds/issue/drupal-3447611/core/tests/Drupal/Tests/WebAssert.php:158
    /builds/issue/drupal-3447611/core/tests/Drupal/FunctionalJavascriptTests/Theme/ClaroViewsBulkOperationsTest.php:124
    FAILURES!
    Tests: 2, Assertions: 33, Failures: 1.
    

    Now seeing the failing test on the MR. Removing tag

    Believe feedback has been addressed, code change looks fine.

Production build 0.69.0 2024