Problem/Motivation

The ajax petitions should use a throbber to know when the petition is finished. If the server is slow the user can think that the VBO component is not working well.

Proposed resolution

Add a throbber on the ajax petition.

CommentFileSizeAuthor
#22 ajax_loader_vbo.png132.57 KBeduardo morales alberti
#5 vbo-ajax.gif61.13 KBcrzdev

Issue fork views_bulk_operations-3349616

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

Eduardo Morales Alberti created an issue. See original summary.

Eduardo Morales Alberti opened merge request !58

eduardo morales alberti’s picture

eduardo morales alberti
Primary language Spanish
Location Spain, 🇪🇺
commented
Status: Active » Needs review
graber’s picture

graber commented

I think this looks bad but let's wait for feedback from others.

crzdev’s picture

crzdev commented
StatusFileSize
new vbo-ajax.gif61.13 KB

Thanks Graber, can you be a bit more specific about "this looks bad", definitely not for final non technical users as it is an UX improvement, besides using Drupal.ajax wrapper instead jQuery that is being replaced progressively in favor of vanilla JS, so when core implement that, in case wrapper still exists, no change will be required related to main petition itself.
VBO Ajax

graber’s picture

graber commented

So there’s a blink and a throbbed for a fraction of a second that is completely not needed and may be confusing. I understand it may be an improvement when a site is slow but in most cases this request is super fast and it’s rather something no one wants to see.

I think developers should rather improve performance. The only way I could see this released is by being a configurable feature, turned off by default.

crzdev’s picture

crzdev commented

That was a prove of concept site with just VBO & less than 35 nodes, into serious sites with thousands of contents and translations, petitions will be considerably slower, even with several cache layers, at least until that cache is calculated. That is an ajax petition that refreshes the selection counter & is a Drupal standard (you can see exact same behaviour when uploading a file, into views page configuration or when ajax is enabled into final display, or many other components), not having it can be confusing for end user editors as mentioned abobe.
Either way, agree to make it configurable, optional & disabled by default.
Even animation can be added to loader with fade in & fade out transition & practically won't be seen if the petition is too fast.
Thanks for the quick feedback.

graber’s picture

graber commented

Yes, the Ajax petition is great, thanks, I think I’ll merge it soon without a throbber if the setting part will not land here.

crzdev’s picture

crzdev commented
Status: Needs review » Needs work

Great! Moving to Needs work to implement throbber as optional, disabled by default & configurable.

eduardo morales alberti’s picture

eduardo morales alberti
Primary language Spanish
Location Spain, 🇪🇺
commented

Why it should be configurable?

If the website is slow the throbber will appear, and the user will notice that the counter is loading, but if the page is faster, the user will not notice the throbber.
It is the standard for Ajax petitions on Drupal.

graber’s picture

graber commented

@Eduardo #6.

  • Graber committed 04a6b263 on 4.2.x authored by CRZDEV 
    Issue #3349616: Use Drupal ajax petition instead of jquery ajax directly...

Eduardo Morales Alberti closed merge request !58

eduardo morales alberti’s picture

eduardo morales alberti
Primary language Spanish
Location Spain, 🇪🇺
commented

Why the commit is "authored by CRZDEV"? I create the issue and I create the MR with the changes https://git.drupalcode.org/project/views_bulk_operations/-/merge_request...
At least add me as a co-author.

graber’s picture

graber commented

Oh dear, sorry, somehow I looked at the discussion and thought CRZDEV was talking like he was the author in #5 and haven’t looked higher 😬 also MR doesn’t set the author automatically as a patch would. We can’t rewrite git history on drupal.org repos but I’ll think of something to make this right.

  • Graber committed 5149ece4 on 4.2.x 
    Revert "Issue #3349616: Use Drupal ajax petition instead of jquery ajax...
graber’s picture

graber commented

Ok, had to revert and recommit with the correct author. Thanks and sorry again!

eduardo morales alberti’s picture

eduardo morales alberti
Primary language Spanish
Location Spain, 🇪🇺
commented

Thank you Graber!

crzdev’s picture

crzdev commented

Really i was co-author of that commit too, i am comfortable with that type of accreditation (commit and revert), thanks for including that part anyway!

Eduardo Morales Alberti opened merge request !65

eduardo morales alberti’s picture

eduardo morales alberti
Primary language Spanish
Location Spain, 🇪🇺
commented
Status: Needs work » Needs review
StatusFileSize
new ajax_loader_vbo.png132.57 KB

Add an option on the VBO View field for the ajax loader.
Ajax loader

graber’s picture

graber commented
Status: Needs review » Needs work

1. New config schema missing.
2. Let's add that setting to test views with any value.

eduardo morales alberti’s picture

eduardo morales alberti
Primary language Spanish
Location Spain, 🇪🇺
commented
Status: Needs work » Needs review

1. Added the new field to the schema.
2. Added setting to the test views.

Could you review Graber?

eduardo morales alberti’s picture

eduardo morales alberti
Primary language Spanish
Location Spain, 🇪🇺
commented

I am not sure how to update the configuration to add this field on the views with views_bulk_operations field.
I will try to replicate something like the update on the install to add this field to all views.

eduardo morales alberti’s picture

eduardo morales alberti
Primary language Spanish
Location Spain, 🇪🇺
commented

Added update to add the ajax_loader new config to the views with views_bulk_operations.

graber’s picture

graber commented

Code looks good!
Can someone test this?

Scope:

  1. `drush updb`
  2. selecting items with the new config set to TRUE and FALSE
crzdev’s picture

crzdev commented
Status: Needs review » Reviewed & tested by the community

LGTM! (Check added comment it is just an small adjust)

graber’s picture

graber commented
Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

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