Problem/Motivation

I noted some weird issues with the throbber, which is inserted during ajax calls. It destroys somehow the markup during the ajax call - I think the problem is:

.ajax-progress {
  float: left; /* LTR */
}

in system-behaviour.css

-> That way the the div is floated, but that's not cleared so the following divs move up.

Proposed resolution

Stop using "float" for displaying of throbbers and progress bars.

See: http://www.youtube.com/watch?v=GHz-bl_mugw&hd=1 for an example, which you can try yourself at http://drupalexamples.info/examples/ajax_example/simplest

Remaining tasks

  • Needs reviews.
  • Needs testing on IE6.

User interface changes

This patch prevents "jumpy" behavior during AJAX requests.

It also fixes a padding error on progress bars. See http://drupal.org/files/ajax-before-after.png

API changes

Themes and modules that provide throbber or progress bar specific CSS would need to accommodate changes (probably by removing the overriding they've done).

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Jacine’s picture

Can you provide more information about how you came across this? What browser, what theme, what were you doing, etc? Thanks :)

fago’s picture

of course, sry.

I experienced it with ubuntu 9.10 + firefox 3.5 and now with ubuntu 10.4 + firefox 3.6. I ran over this several time while adding #ajax elements to the rules ui, so it to depend on which element one is triggering upon.

E.g. you can see it when you install the latest rules ui (see http://drupal.org/node/660108), add any rule + add an action. The action select makes use of #ajax, when you select it the underlying button comes up beside the select during the AJAX request. You can find the HTML code causing it here: http://drupal.pastebin.com/WMPcavAg

@theme: It appears in Seven as well as in Garland for me.

Jacine’s picture

Status: Active » Needs review
FileSize
898 bytes

Hmm, not having much luck with the Rules module right now. Getting a bunch of page not founds when doing anything other than creating a rule.

Anyway, I assume this just needs a clearfix class. Can you please test it out?

fago’s picture

FileSize
1.13 KB

thanks, but that doesn't help. I experienced the bug with the throbber, not with the progress bar (haven't tried that). However adding clearfix to the throbber doesn't fix it either - to get it working I also had to make .throbber div non-floating too. The throbber is displayed at the right position as the triggering element is floated with the .progress-disabled class.

Attached patch fixes the problem for me.

@rules-installation: Hm, that's strange. For me (most) links work.

fago’s picture

FileSize
1.13 KB

grml, fixed the tab in the .css file.

s.Daniel’s picture

Status: Needs review » Needs work
FileSize
3.13 KB

The throbber has display issues on admin/structure/types/manage/news/display as well. In chrome and safari after pushing the update or cancel button they swap positions. The patch does not really help here as the buttons still switch only (see image).

Also with two buttons the throbbler will never actually look clean as the buttons will jump - is there any other generic approach we could take?
Just a quick thought: http://d7.wobster.net/node/4 However I am not convinced of this idea in particular myself ;)

Jacine’s picture

@fago I'm not really sure why I thought this was progress bar related when you clearly said it was the throbber. Sorry about that. I just checked this out on the content type display settings, and I'm not convinced that removing the float on .ajax-progress-throbber is the right move. Removing the float + applying the clearfix moves the throbber down a line, which is not the intended design. I did try testing again with the Rules module, and it appears that you may have worked around the issue. I didn't notice any problems when creating a rule and adding an action. I think that in general, modules are responsible for clearing their own floats. If you think I'm missing something, let me know.

@s.Daniel, I'm not seeing the buttons drop unless I make my browser window really narrow in Safari/Chrome, but it is happening in IE6/IE7. The blinking buttons are an interesting idea, but they aren't much better since they don't indicate progress. I agree this can probably done in a cleaner way, but the UX team would need to be involved, and it would definitely be D8 material.

mrfelton’s picture

I have a similar issue in a module I am developing. I'm using a submit button with ajax. When the button is pressed, the progress bar appears to the right of the button which is fine, but also, the item below the submit button also jumps up next to the progress indicator.

I think the progress bar needs a clearfix.

See these 2 screenshots.

mrfelton’s picture

Note: I managed to get around this issue by wrapping my submit button in a div with a clearfix:

  $form['update_status_group']['submit'] = array(
    '#type' => 'submit',
    '#value' => t('Update database now'),
    '#prefix' => '<div class="clearfix">',
    '#suffix' => '</div>',
    '#submit' => array('ip2locale_webhostinginfo_ip2locale_update_submit'), // If no javascript action.
    '#ajax' => array(
      'path' => 'ip2locale_webhostinginfo/ajax/test',
      'wrapper' => 'ip2locale-webhostinginfo-ip2locale-update-status',
      'method' => 'replace',
      'effect' => 'fade',
      'progress' => array(
        'type' => 'bar',
        'message' => t('Please wait...'),
        'path' => 'ip2locale_webhostinginfo/progress/'.$update_progress_key,
      ),
    ),
  );

Should we have to do this though? It seems that core shouldn't be doing this weird behavior by default.

David_Rothstein’s picture

FileSize
17.21 KB

I came across an issue while working on the new Views 7.x-3.x user interface that is closely related to this one; I think it can probably be considered the same issue.

The problem isn't just with the throbber itself, but also the element that triggers it. The relevant CSS in core is actually two rules:

.progress-disabled {
  float: left; /* LTR */
}
/* Throbber */
.ajax-progress {
  float: left; /* LTR */
}

Where the "progress-disabled" class gets applied to the element that triggers the AJAX.

The Views UI we have is shown in the attached screenshot. Everything is rendered inline by placing it inside of a <fieldset class="container-inline"> element. The select boxes are AJAX-enabled. So what happens is that as soon as you trigger the select, the select element itself jumps all the way to the left side of the fieldset (but the label doesn't jump), and then jumps back a second later when the AJAX is finished processing.

We're working around it at the moment with the following CSS:

.views-admin .container-inline .progress-disabled {
  float: none;
}

But it would be great if anyone has any good ideas for how to fix it in core :)

fago’s picture

@David_Rothstein: Does #5 fix it for you?

David_Rothstein’s picture

No, #5 doesn't seem to help. (The rule would need to be applied to .progress-disabled also, in order to fix the issue we ran into.)

But unfortunately, it sounds from above like there are reasons not to add either of those float: none rules generically in core.

longwave’s picture

This affects Ubercart as well, causing similar unwanted element movement while the floated throbber or progress bar is displayed. The patch from #5 works for my case, but I can see that different layouts may need different solutions here.

geerlingguy’s picture

Yeah, yuck. Just hit this on a few different forms I'm setting up. I had a background color set on the form, and I had to set the form's overflow to 'auto' to contain the now-floated button that triggered the throbber.

Another way that I prevented the strange behavior was by adding some of the following rules, depending on where the throbber was popping up:

.ajax-progress {
  width: 120px; /* Had to set explicit width sometimes to get it to display in one line */
  float: right; /* Fixed the layout for some places where it was displayed */
  display: inline; /* This worked with float: none; depending on what form I was building */
}

Annoying behavior, to say the least, but I don't know what's best here.

Jacine’s picture

Version: 7.x-dev » 8.x-dev

This is a place where a style guide explaining how it's supposed to be used could really be useful. We could start by pulling up all the places this is used in core with summaries of how they're being used (inline, floated vs. absolutely positioned, etc) with screenshots. Then hopefully we can try to make the usage more streamlined and possibly provide a few options out of the box for how to display them.

magnusk’s picture

I tried "float: right" on .ajax-progress but then it was moved too far away from user eyes and interaction. I ended up using "position: absolute" with background and some width+padding, so that in my case it ends up overlapping a dynamically changing paragraph.

Being able to specify a throbber wrapper id would give the most flexibility and precision in positioning, similarly to where the callback's output goes.

quicksketch’s picture

Component: markup » CSS
Status: Needs work » Needs review
FileSize
169.29 KB
1.66 KB

This problem has been on my personal todo list for ages. The use of "float" in our CSS here is completely unnecessary and really bothersome for modules. I run into this problem every time I use #ajax. I made a video for demonstration: http://www.youtube.com/watch?v=GHz-bl_mugw&hd=1

Or you can try it yourself at: http://drupalexamples.info/examples/ajax_example/simplest

This approach gets rid of the "float" entirely. I was responsible for putting it there in the first place (way back in Drupal 6's AHAH framework), but that was before merlinofchaos came up with the better approach that is now used in Views, CTools, and Flag module of using padding on the throbber instead of floating. This patch mimicks the approach used by those modules.

In fact we're already using this approach in core as part of the File module, which overrides core's system.base.css to use this approach(!) which is a bit silly. With this approach we can remove File modules specific overriding of System.

Attached is a before/after screenshot of how this affects core (very minimal). The big difference here is for contrib.

quicksketch’s picture

Issue summary: View changes

Using summary format.

jenlampton’s picture

Status: Needs review » Reviewed & tested by the community

Patch applies cleanly, CSS updated. Much cleaner appearance.

This throbber float problem is a real nuisance in D7 (particularly with features), adding tag for backport.

jenlampton’s picture

And a D7 patch :)

catch’s picture

Version: 8.x-dev » 7.x-dev
Status: Reviewed & tested by the community » Patch (to be ported)

Yep nice change. Committed/pushed to 8.x.

geerlingguy’s picture

Status: Patch (to be ported) » Needs review

Back to testbot, please... (#19). [Edit: How to override 'ignored'? ...]

andypost’s picture

droplet’s picture

testbot

quicksketch’s picture

Status: Needs review » Reviewed & tested by the community

Patch applies and works great on D7 too.

tim.plunkett’s picture

Issue tags: +Needs backport to D7

Fixing tag.

David_Rothstein’s picture

I tried the D7 patch in a few locations and it seemed to work really well.

Just wondering about this comment in the issue summary:

API changes

Themes and modules that provide throbber or progress bar specific CSS would need to accommodate changes (probably by removing the overriding they've done).

Does anyone who has a good understanding of this patch have any specific details on what the above would entail? I.e., do we know of cases where things would break badly if they don't do this?

(I was comforted by the fact that if you take the patch and get rid of the changes to the File module - so that the overrides are still there - everything still seems to look good, at least in that case.)

David_Rothstein’s picture

Oh, also, has anyone tested this patch in Internet Explorer (particularly on the older versions of that browser which we all really love to support)?

droplet’s picture

Status: Reviewed & tested by the community » Needs review
Issue tags: +Novice, +Needs manual testing
FileSize
1.64 KB

Indeed. added IE supports.
IE7 and below doesn't support inline-block. use inline instead.
Sadly, I just deleted my IE6 virtual machine accidentally.

I think it will not "break badly if they don't do this".
eg. they may used "position:absolute" to fix it, element still no changes.

quicksketch’s picture

I don't think we need the specific IE hack since padding works equally well on inline and inline-block elements. We should just be able to make .ajax-progress be inline for everything I think (though I may be wrong). Probably deserves more testing. Though the inline-block line is not an addition, it's already present in D7 today, so we're not introducing any new breakage here, IE6/7 might be broken already today.

David_Rothstein’s picture

Though the inline-block line is not an addition, it's already present in D7 today, so we're not introducing any new breakage here, IE6/7 might be broken already today.

If we can confirm that's the case, then I'm happy to commit the earlier patch instead, and we could leave the IE6/7 fix for a followup... or never :)

Mainly I just wanted to make sure this patch doesn't cause any new problems (regressions) in the IE6/IE7 behavior.

droplet’s picture

Nope. We added display: inline to file.css and now removed from patch #23 /D8 patch. That's why it needed IE hacks. (tested under IE7 )

If we want kill IE hack, we can use inline for all browsers in D7.

(IE8 supported inline-block, so no change to D8)

I have no time to rebuild my IE6 env. can't test it now :(

capellic’s picture

Any update on this? I developed a module that lays out the form (label/field/description) horizontally and this behavior is causing real issues.

capellic’s picture

Issue summary: View changes

Adding before-after screenshot.

mgifford’s picture

28: ajax-769936.patch queued for re-testing.

Victor Safronov’s picture

Issue summary: View changes

Is it possible to backport this into D6? There is a same trouble.

mgifford’s picture

I don't think that folks will be back-porting anything other than security releases to D6 any time soon. Sorry @Victor Safronov.

heddn’s picture

Issue summary: View changes
Issue tags: +ie6

  • catch committed ed3c7d8 on 8.3.x
    Issue #769936 by fago, Jacine, quicksketch, jenlampton: Fixed .ajax-...

  • catch committed ed3c7d8 on 8.3.x
    Issue #769936 by fago, Jacine, quicksketch, jenlampton: Fixed .ajax-...

  • catch committed ed3c7d8 on 8.4.x
    Issue #769936 by fago, Jacine, quicksketch, jenlampton: Fixed .ajax-...

  • catch committed ed3c7d8 on 8.4.x
    Issue #769936 by fago, Jacine, quicksketch, jenlampton: Fixed .ajax-...

  • catch committed ed3c7d8 on 9.1.x
    Issue #769936 by fago, Jacine, quicksketch, jenlampton: Fixed .ajax-...