Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[UX][D8] VBO: Prevent validation errors when user fails to select items and/or action #4702

Open
klonos opened this issue Oct 13, 2020 · 24 comments · May be fixed by backdrop/backdrop#3413
Open

Comments

@klonos
Copy link
Member

klonos commented Oct 13, 2020

This is related to #4701

image

Long-time Drupal/Backdrop users may be familiar with how bulk operations work, but for new users it is not immediately apparent that the "select action" select menu and the "Execute" button are to be used with the checkboxes to the left of each result in the listing (and the fact that there is no header in the checkbox column to explain what the checkboxes are for makes that worse).

I realize that "Apply to selected items" is way more wordy than "Execute", but it clearly conveys the message of "first select which items the action is to be performed on". Compare the above screenshot from Backdrop, to this from D8:

image

@klonos
Copy link
Member Author

klonos commented Oct 13, 2020

...besides, I personally never liked the "Execute" label for that button 😅

@klonos
Copy link
Member Author

klonos commented Oct 13, 2020

...making this change would help us avoid this:

image

@klonos
Copy link
Member Author

klonos commented Oct 13, 2020

...perhaps we should also make it so that the select menu is disabled/locked until at least one checkbox is ticked 🤔

@indigoxela
Copy link
Member

...besides, I personally never liked the "Execute" label for that button

Hehe, I'm curious: why?

"Apply to selected items" is way more wordy

That's true. Wouldn't "Apply" be sufficient?

perhaps we should also make it so that the select menu is disabled/locked until at least one checkbox is ticked

I have mixed feelings regarding that. It might (or might not) cause some head scratching as - to my knowledge - we don't do that elsewhere in Backdrop. Or do we? At least in views.

@klonos
Copy link
Member Author

klonos commented Nov 19, 2020

Hehe, I'm curious: why?

Because execute also means "to kill":

image

It's similar to the reason why we say "user accounts" instead of "users", and #4451

@indigoxela
Copy link
Member

So we better use something like "Go", "Do", "Run", "Apply",...

@klonos
Copy link
Member Author

klonos commented Nov 19, 2020

Yup, also:

"Apply to selected items" is way more wordy

That's true. Wouldn't "Apply" be sufficient? ...

Here's the UX issue here:

  1. User selects an action from the dropdown, since they are being prompted to do so with "choose an action"
  2. They assume that the action will be applied to all items, so they click "execute" -> the page reloads, we throw an error, and the form is reset
  3. So the user now needs to repeat steps 1 and 2, and they have a "how could I know" moment of frustration 👎

Here's what could be done better:

  1. User selects an action from the dropdown, since they are being prompted to do so with "choose an action" -> a warning pops up, prompting them to also select some items
  2. They click "apply to selected items" -> the page reloads, there are no errors, job done 👍

@klonos
Copy link
Member Author

klonos commented Nov 22, 2020

OK, here's what I have so far: backdrop/backdrop#3407

When a view with bulk operations loads, the "Apply" button is hidden, so to avoid accidental clicks caused by missing to select an action and/or items (which lead to errors anyway). At the top of the list/table below, there's a "You need to select at least one item..." prompt:

image

Once at least one item is selected from the list, the "Apply" button is shown, but if no action was selected, it is disabled:

image

The "You need to select at least one item..." prompt is now replaced with a "No action selected." message. If an action is selected, that message goes away, and the "Apply" button is now enabled:

image

I think that this is overall better UX, as there are no errors and no useless page loads if a user has missed to select items/action.

Notes:

  1. I have NOT removed the current validation errors. I think that we should leave those in (for when no js is available).

  2. What I need help with is the "select all" checkbox. I've added a name="select_all" to it, so that it can be targeted with #states, but I couldn't get it to work 😓 ...I believe that this has to do with the fact that this checkbox is added as markup in tableselect.js - it is not a "proper" FAPI element, so #states doesn't work on it.

  3. There is another "Execute" button in admin/config/urls/path/bulk-update but the code for that is separate to the VBO. If people like the approach here, I can either include the same changes to that form, or file a follow up issue to deal with that separately.

@indigoxela
Copy link
Member

indigoxela commented Nov 22, 2020

One thing baffled me: I need to do two things to see and use the submit button: choose an item and choose an action.

I'm not convinced that this is actually an improvement. At least, for me there was a short WTF moment.
no-button-wtf

As currently there are many related tests failing - back to "needs work".

What I need help with is the "select all" checkbox

I probably can't help with that.

@indigoxela
Copy link
Member

BTW, I'd suggest to update the issue title and description. Its initial purpose was a change in wording, but the PR goes beyond that.

@klonos klonos changed the title [UX][D8] VBO: Change "Execute" to "Apply to selected items" [UX][D8] VBO: Prevent validation errors when user fails to select items and/or action Nov 22, 2020
@indigoxela
Copy link
Member

I wonder if the states system allows to let a form item depend on the status of two or more other form items. (I don't think I ever did that.)

The thing that baffled me was that I didn't see the Apply button. (I chose an action first.) If I could see it, but it's disabled until both choices are made (items and action), it wouldn't have caused head scratching.

@olafgrabienski
Copy link

In my opinion, the phrase "You need to select at least one item before any action may be applied" is too demanding for people who don't try to use the actions but do other things on that page. If we need such a message, a more neutral statement like "No item selected" should be fine.

And can we show always the button, disabling it instead of hiding it when the requirements aren't met?

@klonos
Copy link
Member Author

klonos commented Nov 25, 2020

I wonder if the states system allows to let a form item depend on the status of two or more other form items. (I don't think I ever did that.)

Yes it does, and that's what I'm doing in the PR 😉 ...here's one of my favorite articles that explains how multiple conditions (with AND/OR/XOR) work: https://www.lullabot.com/articles/form-api-states

@klonos
Copy link
Member Author

klonos commented Nov 25, 2020

Thanks for testing and providing feedback @indigoxela and @olafgrabienski 🙏 ...I started typing the reasoning why I implemented things the way I did, but it turned out to be an essay on certain #states limitations that prevented me from implementing things a different way. I have instead filled an alternative PR that handles things differently: backdrop/backdrop#3413

  • there are no messages
  • the submit button has 3 different "states":
    • disabled, with "No items selected" as label
    • disabled, with "No action selected" as label
    • enabled, with "Apply to selected items" as label
  • the button labels change*, depending on whether items/actions are selected or not

In general, the issues here are:

  1. It is not obvious that you need to tick the checkboxes next to the items in the list in order to apply actions (there is no label in the column header etc.)
  2. People assume that they have selected items, when they have in fact just filtered items.
  3. It is not ideal to have a button with a label of "Apply to selected items" when there are no items selected. This leads to some people assuming that there are items already selected (see previous point).
  4. In the Seven admin theme, the style of disabled vs enabled secondary submit buttons is very similar (gray background, with black/gray font). If you disable the button, people may wonder why it is "not working", because they think that they have selected items. This becomes worse on devices without mouse pointers, where you can not see the hover-over green button effect.
  5. Given points 1, 2, 3, and 4 above, you may end up with people assuming that the "filtered" set of results is what we call "selected", and they try to click the submit button. So:
    • advanced users, or people with Drupal/Backdrop experience do this:
      1. filter content ->
      2. select items (because they know that this is a thing) ->
      3. select action ->
      4. click the submit button -> everything works as expected, without any errors 👍
    • less technical people, or people without Drupal/Backdrop experience do this instead:
      1. filter content ->
      2. select action ->
      3. try to apply action (because they think they have selected items) -> get validation errors 👎

(*) Sine you cannot control the label of buttons via #states. I have instead used 2 "pseudo" buttons + the actual submit button. I make sure that only 1 of the buttons is shown at each given point, by showing/hiding the others via #states. I don't like this approach as much from a DX point, but it has a better UX (no "WFT, where's my button" effect that @indigoxela mentioned).

@klonos
Copy link
Member Author

klonos commented Nov 25, 2020

Here's how this works in the alternative PR:

pseudo-submit-buttons-small

@indigoxela
Copy link
Member

indigoxela commented Nov 25, 2020

That's a fancy approach! With Javascript enabled it works like a charm (feels a bit like "guided deciding").

One thing I noticed, though, is that the form looks pretty weird with Javascript disabled. Not sure, how to deal with that.

all-buttons-visible

@klonos
Copy link
Member Author

klonos commented Nov 25, 2020

See how I said:

...I don't like this approach as much from a DX point

@klonos
Copy link
Member Author

klonos commented Nov 25, 2020

...perhaps I can add .js-hide classes to these buttons. Let me give it a try.

@klonos
Copy link
Member Author

klonos commented Nov 25, 2020

...PR updated to hide the pseudo-buttons when no js.

@docwilmot
Copy link
Contributor

Cant we just use JQuery to change the labels and/or disable the button instead of states?

@klonos
Copy link
Member Author

klonos commented Nov 25, 2020

Yes we can @docwilmot ...but I'm not good at it 😓 ...in the meantime, my PRs can serve as a way for people to compare the UX before/after. Once we have settled on the best approach, someone else with good JS skills may take over this issue and implement things in jQuery.

@indigoxela
Copy link
Member

indigoxela commented Nov 26, 2020

my PRs can serve as a way for people to compare the UX before/after

I gave it another try - with and without Javascript. It works and the UI doesn't look cluttered anymore without JS. 👍

Remaining tasks:

  1. The disabled buttons are still there in markup - only hidden. It's probably better to use a JS approach that adds/replaces buttons dynamically (as @docwilmot suggests).
  2. Fix the tests in comment.test and user.test to seek for the new button text ("Apply...")
  3. Fix the "select all rows" checkbox handling - it currently doesn't trigger the button switch

@docwilmot
Copy link
Contributor

I cant do a PR now, have to go proofread my wife's dissertation, which I should have been doing for the last hour instead of this, but here's some code that works:

Backdrop.behaviors.viewsUiBulkForm = {
  attach: function (context) {
    var $context = $(context);
    var $applyButton = $('#views-bulk-form-apply');
    var buttonText = $applyButton.val();
    $applyButton.attr('disabled', 'disabled').addClass('no-js-hide form-button-disabled');
    $('.form-checkbox').on('click', function () {
      toggleExecuteButton();
    });
    $('#edit-action').on('click', function () {
      toggleExecuteButton();
    });
    function toggleExecuteButton (event) {
      var $rowChecked = $context.find('.form-checkbox:checked').length;
      var $actionSelected = $('#edit-action').val();
      if ($rowChecked && $actionSelected) {
        $applyButton.attr('disabled', false).removeClass('no-js-hide form-button-disabled').prop('value', buttonText);
      }
      else {
        var newButtonText = $rowChecked ? Backdrop.t('No action selected') : Backdrop.t('No item selected');
        $applyButton.attr('disabled', 'disabled').addClass('no-js-hide form-button-disabled').prop('value', newButtonText);
      }
    }
  }
};

I added that to views_ui/js/views-admin.js and included it in the form function with $form['#attached']['js'][] = backdrop_add_js(backdrop_get_path('module', 'views_ui') . '/js/views-admin.js');, but it could probably go into Views module instead of Views UI? Not sure.

Also need to add an ID to the submit button:

      '#type' => 'submit',
      '#id' => 'views-bulk-form-apply',
      '#value' => t('Apply to selected items'),
    );

@klonos klonos assigned klonos and unassigned klonos Nov 26, 2020
@klonos
Copy link
Member Author

klonos commented Dec 18, 2020

Thanks so much @docwilmot 🙏 ...it's easier for me to tweak js others write, than produce it myself from scratch 😅

I've got it to work:

  • .on('click', wasn't working as expected (the disabled state and label of the button would randomly change at the moment you clicked on the dropdown - even if you did not select any value). .on('change', on the other hand worked nicely 😉
  • I had to initialize the Apply button when the form would load (using toggleExecuteButton() once), otherwise it would load disabled, with its text set to Apply to selected items (which would defeat the purpose).
  • I called the behavior viewsBulkFormApplyToggle, but happy to rename to whatever makes sense, based on feedback from other.
  • moved code around a bit.

...it could probably go into Views module instead of Views UI? Not sure.

It should. Otherwise this wouldn't work at all if Views UI got disabled (some site builders do that after they've built their views). I've put the code in a new core/modules/views/js/views_bulk_actions.js file. Happy to change the name of the file based on feedback.

...included it in the form function with $form['#attached']['js'][] = backdrop_add_js(...)

No need to wrap the path around backdrop_add_js().

PR updated and ready for review/testing: backdrop/backdrop#3413

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment