While editing block positions using the overlay module, the table header for the page does not follow like it does when you are not using the overlay so you know exactly what column you are looking at. I am unsure of what the actual word/function name for the following table header is so hopefully this makes sense.

Comments

triversedesigns’s picture

Update: this also does not work on the Permissions page as well. It seems to be all table headers.

Ryan Palmer’s picture

Sticky table headers uses the css position: fixed; to place table headers when scrolling beyond the table header.

http://www.w3schools.com/Css/pr_class_position.asp

fixed | Generates an absolutely positioned element, positioned relative to the browser window.

Perhaps the solution here is to use "absolute" instead of "fixed" for the tableheader. I haven't tried this, but I know could never get sticky table headers to work in overlays on Drupal 6.

casey’s picture

Subscribing.

casey’s picture

Hmm this is a difficult one. Because of the autoFit functionality for the overlay there never are scrollbars in overlay's iframe window.

kiphaas7’s picture

Yet another consequence of using an iframe. As casey pointed out, the overlay makes sure the iframe never gets scrollbars, and since the iframe contains an entire separate document, the stickyheader never get's activated, because the document in the iframe never shows scrollbars. So either we make sure the iframe has scrollbars, or we make sure that the sticky header code works with our overlay. (I prefer the first approach).

kiphaas7’s picture

Third option would be adding extra JavaScript behavior to our overlay code so that tableheaders work again, by binding the scroll event of the mainwindow, then calculate the scrollOfset of the main window, the offset of the iframe against that main window, and set the difference of those 2 as an offset for the absolutely positioned stickyheader.

That option would make more sense than adding extra scrollbars. Thoughts?

casey’s picture

Yea that should do. But using JS for position of stickyheader doesn't have the smooth effect that position:fixed in CSS has.

We could also try to copy those stickyheaders to the parent window inside the #overlay-container (this one needs position:relative then) just above overlay-iframe.

kiphaas7’s picture

Position: relative won't work as you imagine it would, position fixed is relative to the browser window, so position relative wouldn't help. http://www.quirksmode.org/css/position.html

Taking it outside the iframe opens a whole new can of worms: the offset needs to be calculated, the width needs to resized with every window resize.... I think it's less costly to keep it in the child frame and manipulate it there.

kiphaas7’s picture

Status: Active » Needs review
StatusFileSize
new2.28 KB

First attempt at restoring sticky tableheaders.

casey’s picture

Didn't test it yet, but should it be based on height of elements with the "overlay-displaced-top" class instead of just the toolbar?

kiphaas7’s picture

It get's the offset from the wrapper element, and the wrapper indeed calculates for overlay-displaced-top. I'm only doing an explicit calculation for the shadow of the toolbar, so that the shadow falls over the table header.

----

That said, performance wise it would be better to move some code around

This:

+    if($stickyTables.length) {
+      $(window).bind('scroll.overlay-event', function() {
+        // Get the parent offset and scrollposition.
+        var parentScroll = $(this).scrollTop();
+        var parentOffset = self.$wrapper.position();
+
+        // Account for the toolbar shadow, if there is one.
+        var shadow = $('#toolbar .shadow');
+        var shadowHeight = (shadow.length) ? shadow.height() : 0;

To:

+    if($stickyTables.length) {
+      // Account for the toolbar shadow, if there is one.
+      var shadow = $('#toolbar .shadow');
+      var shadowHeight = (shadow.length) ? shadow.height() : 0;
+
+      $(window).bind('scroll.overlay-event', function() {
+        // Get the parent offset and scrollposition.
+        var parentScroll = $(this).scrollTop();
+        var parentOffset = self.$wrapper.position();

My dev pc is currently undergoing some maintenance, so can't reroll it myself for now :).

reglogge’s picture

Status: Needs review » Needs work
StatusFileSize
new16.91 KB
new24.27 KB

The patch in #9 only partially works for me:

- no effect in Safari4/Mac
- table headers only partially visible behind shortcuts bar in FF3.5/Mac (see screenshot sticky_headers.png)
- table headers only partially visible without shortcuts bar in FF3.5/Mac (see screenshot sticky_headers_wo_shortcuts.png)

Haven't tested in other browsers yet.

aspilicious’s picture

This need some love!

BarisW’s picture

StatusFileSize
new58.04 KB

It works great on the Permissions page but not on other tables. Could it have something to do with the height of the THEAD (because the table header is higher on the permissions page due to a word-wrap)?

Tested on Firefox 3.6.3 on MacOSX.

webchick’s picture

Priority: Normal » Critical

As much as I hate to do this, this is really a crappy bug that we need to fix.

tstoeckler’s picture

Title: Overlay Blocks Editing Auto Follow Table Header » Sticky tableheaders don't work in Overlay

Retitling.

kiphaas7’s picture

Status: Needs work » Needs review
StatusFileSize
new2.32 KB

Let me start off by saying that I don't really like my own patch, and that it's not finished (see @todo in patch and comments below). Allthough it brings back the auto-follow in the browsers I tested (IE 8, Opera 10.5, FF3.6, Chrome, Safari 4), it does that with some downsides.

Quirks in the patch:

  • self.$iframe.load(function() { does not fire at all in opera 10.5. Big wtf for me, and a quick google didn't resolve anything. Could be a jquery bug, could be an opera bug. self.$iframeDocument.ready(function() { does fire in all tested browsers, but needs a rather raunchy timeout function (at least I think so...).
  • The shadow implementation of the toolbar changed since the last time I rolled a patch for this issue, and made it quite a bit harder to figure out, since it's now all CSS3 (or filter for IE). For now, I just hardcoded it to 20px, which coincides with the CSS box-shadow height. Yes, it is a bit off in IE, but not that much.
  • It feels like (IMHO) way too much code to fix something like this.
  • The technique used (binding the main window scroll event to the iframe), significantly jitters the scrolling experience in some browsers...

.....

But it does work! :)

Status: Needs review » Needs work

The last submitted patch, sticky.patch, failed testing.

kiphaas7’s picture

One other way would be to clone the original table outside the iframe, and apply the behavior there. Would stop the jerkiness while scrolling (since the scoll event is no longer abused)

I tried and failed in that approach, need someone who is more capable for this issue...

cwgordon7’s picture

Assigned: Unassigned » cwgordon7

I'll probably fix this over the next few days.

kiphaas7’s picture

Nice, looking forward to your solution!

cwgordon7’s picture

Ok, so here are our options as I see them:

1. Manually adjust the css "top" property to keep the tableheader where it should be on the iframe. Problem: this is clunky, and shows up poorly in the browser (e.g. patchy/laggy), particularly in slower browsers.

2. Take the tableheader outside of the iframe, and then keep it on top at a constant point on the page, like we currently do with the toolbar. Problem: this would change the theming of the tableheader in the case that there is a different theme within the overlay. Solution: Put it in its own iframe with the correct theme. Problem: this is ugly and complicated. Secondary problem: there's no easy way to solve alignment issues if we go down this path.

3. (my favorite) Change how the overlay works. Currently we make the overlay as big as it needs to be and let the page scroll. I think this approach is problematic, not only for the tableheader but for some other situations too, such as when you open the overlay when you're already on a long page and you're left with a large shaded-out region at the bottom of the page. The solution would be to disable page scrolling when the overlay opens (body { overflow-y: hidden }), and instead have the overlay iframe itself scroll. I personally think that this would be the most intuitive scrolling mechanism anyway, and would allow us to solve the tableheader issue without much trouble.

Discussion welcome. I'd particularly like ksenzee to look at this.

cwgordon7’s picture

Alternatively, for #2, I think we don't have to use an iframe if we use the jQuery calculated CSS plugin (see http://github.com/peol/jquery-computed-style), which I think would allow us to apply the calculated CSS properties of the tableheader within the iframe directly onto the tableheader outside the iframe.

kiphaas7’s picture

#23:
1 Is what I tried. Clunky it is.

2 Is IMHO the best option for now. Together with something like you mentioned in #24, it would be usable. The clunkiness of 1 would be solved by using position:fixed. Only problem left is calculating for what distance the header needs to be shown.

3 is not really an option. You can't hide the main vertical scrollbar in all browsers, and that would mean 2 scrollbars, which sucks. I already tried binding the scroll event of the main window to the iframe, but that is also clunky.

casey’s picture

@Kiphaas7 "You can't hide the main vertical scrollbar in all browsers, and that would mean 2 scrollbars, which sucks"

Could you give me an example?

cwgordon7’s picture

@Kiphaas7 Why can't you get rid of the vertical scrollbar in all browsers just by reducing the size of the content so no scrollbar is necessary? If we need a browser-specific trick, we can do it (e.g. hide main page content when the overlay opens in IE6 / etc.).

kiphaas7’s picture

Err, I was sure it couldn't be done cross browser. I just checked, and it seems possible. Sorry :/

But: if the scrollbar of the main window gets hidden, and the iframe scrollbar is used, doesn't that mean the iframe scrollbar can't be stretched over the entire main viewport (as one would expect), because of the toolbar overlaying the iframe?

kiphaas7’s picture

I'm pretty close to a fix while taking path #2. Needs some more love though, will post when done.

Path 3 would be more ideal, but I thought I saw a patch by casey somewhere doing something like #3, I tried approach #2.

kiphaas7’s picture

Status: Needs work » Needs review
StatusFileSize
new7.94 KB

Initial try at restoring sticky tables according to method #2 suggested in #23.
Works OK in safari and Firefox, apart from graphical errors also good in IE8, but extremely buggy in opera 10.53 for reasons beyond me.

Needs a thorough review and thoughts whether this is the way to go.

Personally, thinking more and more that method #3 in #23 might be better, if the scrollbar issue can get worked around.

Anyone up for making a patch using method #3?

EDIT: didn't use the plugin suggested in #24 because it somehow didn't produce reliable results crossbrowser for me...

cwgordon7’s picture

+      delayedOuterResize;

Shouldn't that be:

+      delayedOuterResize();

Should Drupal.overlay.getCSS be Drupal.overlay._getCSS to signify that it's an internal function? Or furthermore, should that be moved to drupal.js? Also that array needs a bit of formatting work to comply with coding standards. Are those all the possible CSS properties there? Any reason for the ordering of those properties, or should they be alphabetized?

Lastly, statements like while(i--) and if(i = self.stickyTables.length) need spaces between the while/if and the opening parentheses.

Other than that, looks like a solid proof of concept. I'll be happy to update this with the required complex resizing code and/or track down bugs in particular browsers.

MichaelCole’s picture

Status: Needs review » Needs work

Marking "needs work" based on comments in #31.

How would you reproduce/test this?

Is this really a critical bug? No crash, or data corruption. Workaround would be to scroll back up right? Am I missing something?

webchick’s picture

I feel it's critical because it effectively breaks a major user-facing feature that worked just fine in D6.

kiphaas7’s picture

just thought of this: A very simple workaround would be instead of using auto following table headers, to just insert the tableheaders every n rows in the table itself....

@31: thanks for the comments, will work those in a new patch.

That array of css values certainly does not contain all possible values. I got it somewhere from stackoverflow I think, and at least for the array part, was pretty much copy paste.

Still nobody wanting to try method 3? I'd hate not having explored that path before comitting to this approach.

casey’s picture

method 3 needs a lot of things done differently. #668640: Overlay shouldn't be based on jQuery UI Dialog uses method 3.

kiphaas7’s picture

I just tried #668640, and IMHO it's way more robust, and needs less code (and sticky tables are already functional there!).

Yes it needs more changes, but would be better in the long run.

I'd like to hear some consensus from people higher up in the chain what's the status on these 2 issues: if #668640 has a good chance of being committed for Drupal 7, then it's not worth pursuing this (IMHO technical inferior) issue. I'd rather try and help finish #668640.

I tried in my patches in this issue to emulate the original conditions for sticky table headers, in #668640 the original conditions are restored. #668640: Overlay shouldn't be based on jQuery UI Dialog is the high road :).

cwgordon7’s picture

The usability people don't like approach #3 because of the "scrollbar within a scrollbar" problem. Since the entire point of the overlay is supposed to be to improve usability, I'm inclined to listen to the usability people on this one. We could just make the tableheader not scroll down, of course, but that's also a degraded experience.

I think the best idea is to go with the solution in #30 - not terribly elegant, but workable. I'll be playing around with this a bit tonight.

cwgordon7’s picture

Status: Needs work » Needs review
StatusFileSize
new8.45 KB

Here's an updated version of the solution in #30. It now works with resizing and toolbars and stuff, plus a little bit of code cleanup. Possibly ready for a thorough review.

Status: Needs review » Needs work

The last submitted patch, sticky-tables-overlay-alternative-2-02.patch, failed testing.

cwgordon7’s picture

Status: Needs work » Needs review
StatusFileSize
new8.54 KB

Let's try that again...

cwgordon7’s picture

bump? :)

aspilicious’s picture

Let's wait a little more. If #668640: Overlay shouldn't be based on jQuery UI Dialog gets committed this one is useless.

yesct’s picture

I just marked #668640: Overlay shouldn't be based on jQuery UI Dialog rtbc. Should it be critical if it fixes this critical issue?

yesct’s picture

yoroy’s picture

Only local images are allowed.

Works in FF, Chrome and Safari under OS X

yesct’s picture

casey’s picture

Please review #668640: Overlay shouldn't be based on jQuery UI Dialog... This issue is fixed when that one lands.

webchick’s picture

Status: Needs review » Fixed

Sweet! Confirmed! Had to clear cache first.

Status: Fixed » Closed (fixed)

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