When switching between pages in the overlay the shortcut-add-remove-link and tabs are visible while loading the new page.

Solution is simple: remove shortcut-add-remove-link and tabs in Drupal.overlay.load instead of Drupal.overlay.bindChild

CommentFileSizeAuthor
#22 overlaysecondarytabs.patch3.28 KBcasey
#21 overlay_fix3.patch2 KBaspilicious
#20 overlay_fix2.patch1.76 KBaspilicious
#19 overlay_fix.patch1.76 KBaspilicious
#17 situationSubtabsNotWorking.png190.19 KBaspilicious
#17 situationSubtabsWorking.png245.27 KBaspilicious
#10 overlaykeeptabs.patch1.7 KBcasey
#7 overlaykeeptabs.patch1.17 KBcasey
#3 overlayloading.patch1.67 KBcasey
#1 overlayloading.patch1.66 KBcasey

Comments

casey’s picture

casey commented
Status: Active » Needs review
StatusFileSize
new overlayloading.patch1.66 KB
dries’s picture

dries commented

Not sure I fully understand this bug report, or how to reproduce it. Care to elaborate?

casey’s picture

casey commented
StatusFileSize
new overlayloading.patch1.67 KB

Patch doesn't apply any more.

Example:
- open "Content" from toolbar (has shortcut-add-remove-link and tabs) and wait till loaded
- open another, e.g. "Structure"
- during "Loading..." you still see the shortcut-add-remove-link and tabs from "Content"

aspilicious’s picture

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

Ok works in every browser.
Looked at the code, it's just some lines moved around.
Making this RTBC.

Bojhan’s picture

Bojhan commented

Ahh, yup! This has always annoyed me. Its like leaving old stuff on the screen for just a bit.

webchick’s picture

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

Hm. Another question is why is there shortcut module code in overlay's js to begin with? But I've committed this patch to HEAD, since I can confirm that it solves the bug, and is indeed just moving code around.

casey’s picture

casey commented
Status: Fixed » Needs review
StatusFileSize
new overlaykeeptabs.patch1.17 KB

If the clicked link is one of the tabs, we shouldn't remove those tabs as they are going to re-appear.

Patch is dependant on the one in #683128: Multiple 'add to shortcut' links appearing when adding fields; that one should go in first!

aspilicious’s picture

aspilicious commented

At first sight this looks good (cause it was my idea :p).
But im gonna let it review by other people.

aspilicious’s picture

aspilicious commented

Hmm maybe i see another improvement. If you click on a tab, then it starts loading and after that it becomes your "primary" tab. I think it's better if it becomes your primary tab when it starts loading. I tried to copy some code as proof of concept but it didn't worked.

Maybe someone else can do this?

casey’s picture

casey commented
StatusFileSize
new overlaykeeptabs.patch1.7 KB

Whatever you wish :)

What this patch is doing is easily tested when switching between tabs on "Content" (admin/content) in toolbar.

aspilicious’s picture

aspilicious commented

:) good work!

casey’s picture

casey commented

This is a ux improvement and ready to go in. plz review.

Bojhan’s picture

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

It worked here, a nice detail.

dries’s picture

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

Nice improvement. Committed to CVS HEAD.

dries’s picture

dries commented
Status: Fixed » Needs review

I'm going to re-open this because it seems to introduce inconsistent behavior. If you click on a sub-tab, the tabs will disappear. In other words, we now have a situation where tabs can disappear but not always.

aspilicious’s picture

aspilicious commented
StatusFileSize
new situationSubtabsWorking.png245.27 KB
new situationSubtabsNotWorking.png190.19 KB

Im gonna help this issue with some proper test results.
I attached two screenshots, one is failing, one isn't.
I filtered the cause.

The tabs only stay with a secondary menu, when you click on the first menu item.
In case of my screenshot that is "Global settings"

Hopefully you can use this info!

aspilicious’s picture

aspilicious commented

EDITED: forget the past, look at next post

aspilicious’s picture

aspilicious commented
StatusFileSize
new overlay_fix.patch1.76 KB

This works on my localhost.
I need a code review as this is my first javascript/jquery patch!

aspilicious’s picture

aspilicious commented
StatusFileSize
new overlay_fix2.patch1.76 KB

Wrong patch, damnit

aspilicious’s picture

aspilicious commented
StatusFileSize
new overlay_fix3.patch2 KB

Maybe this own is better.
In this one the active secondary tab switches faster.

Bad side: labels secondary tabs jumping a bit due to changing active style

casey’s picture

casey commented
StatusFileSize
new overlaysecondarytabs.patch3.28 KB

Changes mainly to prevent doing too much.

aspilicious’s picture

aspilicious commented

Thnx casey for reviewing it and making it better...

Hmm are those secondary tabs also jumping a lot on your computer:

1) click on a secondary tab
2) the clicked tab becomes the active one
3) look at the padding before it gets reloaded (you have to be quick ;) )
4) After the page is loaded the right-padding in the active tab is larger.

But this isn't realy related to this issue,

seutje’s picture

seutje commented

seems to work as expected, don't even get the inconsistent padding mentioned in #23

aspilicious’s picture

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

Ok marking this RTBC

casey’s picture

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

http://drupal.org/cvs?commit=339750

Status: Fixed » Closed (fixed)

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