Problem/Motivation

#769936: .ajax-progress divs floating problem is resolved for D8 but we have the following code in views_ui.admin.theme.css

/**
 * Drupal core forces AJAX triggering elements to float left when they are
 * disabled due to AJAX processing. On the add view page, we have inline
 * containers where we don't want that behavior; it causes the select dropdown
 * which is triggered to jump to the left while the AJAX throbber is active.
 *
 * See also http://drupal.org/node/769936 (Drupal core issue); when that is
 * fixed it may no longer be necessary to do this.
 */
.views-admin .container-inline .progress-disabled {
  float: none;
}

Perhaps we can remove it?

Proposed resolution

Remove css.

Remaining tasks

 

 

Contributor tasks needed
Task Novice task? Contributor instructions Complete?
Create a patch Y Instructions
Manually test the patch Y Instructions
Embed before and after screenshots in the issue summary Y Instructions
Review patch to ensure that it fixes the issue, stays within scope, is properly documented, and follows coding standards Instructions

User interface changes

None

API changes

None

CommentFileSizeAuthor
#9 7-9-interdiff.txt1.11 KBalexpott
#9 2315015.9.patch4.43 KBalexpott
#7 2315015.7.patch4.22 KBalexpott
#2 2315015-Remove-AJAX-throbber-jumping-prevention.patch904 bytesgauravkhambhala

Comments

gauravkhambhala’s picture

gauravkhambhala commented
Assigned: Unassigned » gauravkhambhala

I will create patch to remove above code.

gauravkhambhala’s picture

gauravkhambhala commented
StatusFileSize
new 2315015-Remove-AJAX-throbber-jumping-prevention.patch904 bytes

Uploading patch here.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Active » Needs review
Issue tags: +Needs screenshots

@gauravkhambhala in order to get the testbot to review the patch you need to set to needs review. But far important with this one is some sort of evidence that the throbber does not make the views ui jump since that is what the comment is saying that the removed CSS prevents.

gauravkhambhala’s picture

gauravkhambhala commented

@alexpott I didn't really see what has been fixed by removing css in add views page. Please check the this screencast https://www.youtube.com/watch?v=XlENTsYHR-U This has been captured after applying the patch above. Let me know if anything else to test.

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Status: Needs review » Reviewed & tested by the community

This looks good to me, the CSS is no longer required and there is no difference before or after.

There is suspiciously similar but uncommented code in views.module.css:

.view .progress-disabled {
  float: none;
}
gauravkhambhala’s picture

gauravkhambhala commented

@longwave Any action required on the code that you mentioned in views.module.css?

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Title: Remove AJAX throbber jumping prevention CSS from views_ui.admin.theme.css » Remove progress-disabled class and associated CSS
Status: Reviewed & tested by the community » Needs review
StatusFileSize
new 2315015.7.patch4.22 KB

#1989480: Progress Bar style update and #1280674: Submit button "floats" during ajax submission have removed all stylings on of progress-disabled class. Therefore I think we can completely remove it.

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented 
         $(ajaxElements)
-          .addClass('progress-disabled')
           .after($throbber);

This looks strange after the line in the middle is removed. Otherwise I think this is good to go.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Issue tags: +JavaScript
StatusFileSize
new 2315015.9.patch4.43 KB
new 7-9-interdiff.txt1.11 KB

Okay-dokey - in fact the create of the $throbber variable looks unnecessary.

nod_’s picture

nod_
Primary language French
Location Lille
commented

I could see how one would use the class for some custom styling but I'll let a themer say whether that class makes sense to keep or not (because using prop I'm pretty sure css like [diabled] won't work).

As far as JS is concern I'm ok with it. If the class is not needed, feel free to RTBC.

And thanks for the tag :)

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented

I think both [disabled] and :disabled are valid CSS selectors for elements with the disabled property, actually. We already use :disabled in Seven's buttons.theme.css.

nitishchopra’s picture

nitishchopra commented

#9 patch applied succesfully and the instances of "progress-disabled class" are removed.

jOksanen’s picture

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

#9 Applies and the functionality works perfectly. Marking as RTBC. Coding standards are followed.

  • webchick committed 6519a05 on 8.0.x 
    Issue #2315015 by alexpott, gauravkhambhala, longwave: Remove progress-...
webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Status: Reviewed & tested by the community » Fixed

Yay, less code! Good hunting. :)

Committed and pushed to 8.0.x. Thanks!

Status: Fixed » Closed (fixed)

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