| Project: | Drupal core |
| Version: | 7.x-dev |
| Component: | overlay.module |
| Category: | bug report |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | closed (fixed) |
Issue Summary
The loading spinner graphic that appears when loading the overlay content is often not visible because is is displayed outside the viewport. Currently, it is displayed at a position of 50% 50% (right in the middle of the current overlay). If the current overlay is particularity long, the spinner is often displayed out of sight. I find that this looks odd and inconsistent as sometimes I see a spinner and sometimes I don't. I should either see it every time or not at all.
Two solutions I can think of:
1) Display it a set number of pixels from the top of the overlay (eg. 100px) instead of centring it. This would ensure that it is visible all the time.
2) Calculate the size of the viewport and centre the spinner within that. This is probably overkill though.
Attached patch does option 1 and IMO is an improvement as it ensures consistency when clicking around on the top level tabs (click People, then Structure, then Content etc) It is not perfect, as for example, if you scroll down to the bottom of an overlay and click a link, the spinner is now displayed at the top of the overlay - out of sight. However, this is the same situation as without the patch (visibility depends on where you are scrolled to in the overlay). A JavaScript solution more like what I describe in option 2 would be needed to to address that.
| Attachment | Size | Status | Test result | Operations |
|---|---|---|---|---|
| overlayloadinggraphic.patch | 859 bytes | Idle | Passed on all environments. | View details |
Comments
#1
#2
Something like option 2 is pretty simple. A it looks nicer I think.
#3
Oops
#4
Yes, agreed, that is even better.
#5
The last submitted patch, overlayloading.patch, failed testing.
#6
Re-test of overlayloading.patch from comment #3 was requested by casey.
#7
Patch in #3 fixes this issue for me. mrfelton, did you also test it or do you just agree? RTBC then, although it probably needs a re-roll.
#8
Thanks, this looks good to me! Still applies cleanly. Committed to HEAD.
#9
Automatically closed -- issue fixed for 2 weeks with no activity.
#10
Casey notified me that currently there is kind of a basket open/closing effect between each overlay. For example :
1. Structure admin page
2. I click on Content
3. Loading icon
4. The white area quickly collapses and opens to the appropriate size
5. Content page
So my main problem is with 4, it shouldn't collapse it should stay the same size, get bigger or get smaller - it shouldn't flickr to 10% and then go to 65%.
#11
New idea: rollback #3 to prevent the jumping from #10, and fix the position of the spinner with inline styles, set with javascript?
#12
Or just keep overlay content visible while loading... just like other browsers do.
Maybe this needs a spinner next to the "Loading..." title.
Well give this a try... overlay documents will be visible quicker.
#13
Result is even better combined with patch in #674852: Remove shortcut-add-remove-link and tabs in Drupal.overlay.load instead of Drupal.overlay.bindChild.
#14
Reroll
#15
#686926: Keep last page until next page has loaded in overlay instead of spinner
#16
#674852: Remove shortcut-add-remove-link and tabs in Drupal.overlay.load instead of Drupal.overlay.bindChild is committed.
This patch also fixes issues like #690276: 'Weight' column is not displayed and shifts other columns to left
#17
Tested it, and i'm putting this to RTBC.
This looks way better. The only thing we can discuss is when the tabs should dissapear.
Like they do now or do they have to wait untill the page is loaded.
But this sounds more like a follow up patch.
#18
#19
Putting it back, I'm running some tests with other overlay patches
#20
This can go in now after some extra testing!
#21
My experience with this latest patch (FF 3.6, Mac) is that I never see the spinner at all. The only subtle indication that anything happened when I clicked something is the title changes to "Loading..."
I agree it seems to be loading pages faster, which is definitely a good thing, but I think we need that eye-catching animation somewhere (Maybe next to the word "Loading..." ?) so that people don't continue clicking thinking that a particular link didn't do anything.
Make sense, or am I on crack?
#22
I don't know....
I haven't got a good view on it, cause I have full speed internet, running drupal on localhost and got a fast computer so loading takes less then 2 second....
Do you think that users will have the time to click away before the page is loaded.
#23
The patch is committed by accident? Anyway I think it's an improvement.
About the spinner; do we need it? Browsers already show a spinner/loadingbar when loading something. I think that's enough visual indication.
#24
I think extra spinners will make things ugly...
#25
If we keep this patch we need this followup patch to fix an issue since jquery 1.4 is committed.
#26
Great and fast
#27
Btw webchick you already commited patch #14
#28
Oops. :P I didn't mean to do that. :P
Well, anyway, committed the fix in #25.
If the consensus here is to get rid of the spinner, then we should remove that graphic and references to it in the CSS. I still feel like this might need some discussion with the UX team, though. If not, then feel free to move back to fixed.
#29
Last call for UX people :)???
Can someone make a patch adressing comment of webchick (#28)
#30
Apparently there were people that requested this: #686926: Keep last page until next page has loaded in overlay instead of spinner.
If we keep it this way, we can remove the spinner image.
#31
Yippee, +1 to get rid of the spinner rather today than tomorrow :)
It may be a relic from the times when the overlay was loading real slow.
#32
#668640: Overlay shouldn't be based on jQuery UI Dialog landed. Let's close this issue.
There is however no loading animation when opening the overlay, but we should discuss this in another issue.