Download & Extend

[Meta] the big javascript toolbar/tableheader/overlay/fragment mess

Project:Drupal core
Version:8.x-dev
Component:javascript
Category:bug report
Priority:normal
Assigned:Unassigned
Status:active
Issue tags:JavaScript clean-up

Issue Summary

There are several issues for various components that all come down to bad integration with each other. We have:

  • toolbar: this adds a fixed element at the top of the page for links,
  • tableheader: it allows a table header to stick at the top of the page while we're scrolling over a table,
  • overlay: adds an iframe on top of the current page to display administrative pages without getting the user lost in admin (it doesn't use AJAX!),
  • url fragment: a 30 or so years old technology that allow a link to refer to a specific part of an HTML page using an element's id

Over the year we managed to break pretty much everything at different levels.

  1. toolbar + tableheader: both stick elements on top of the page. Elements which overlaps. instead of fixing this all on the js-side, PHP and a javascript eval got involved to compute the offset required to display tableheader properly under toolbar. #1417378: Remove eval() from tableHeader JavaScript. fixed
  2. tableheader + overlay: it works, but not like it should, fixed
  3. toolbar + tableheader + overlay: Since the PHP+eval thing for toolbar did not account for the creation of the overlay module, overlay had to override the function to add it's own value that adds an offset to properly display tableheader under the toolbar, inside the iframe. The fix isn't actually needed — because (for the people who had to deal with this) topOffset is always undefined, what is defined is <strong>parent</strong>.Drupal.overlayChild.prevTableHeaderOffset because it's in an iframe.
    #787670: Clean up tableheader.js interesting stuff in there.
    fixed, using a listener on the 'offsettopchange' event to update the CSS inside the overlay. Some jQuery trickery was needed.
  4. tableheader + fragment: the browser will scroll so that the element with the targeted id shows up at the top of the screen. If you have elements set with position:fixed the fragment targets is hidden behind. #567754: Wrong anchor adjustment in tableheader.js
  5. overlay + fragment: now this is totally broken, since bqq used for ajax bookmarking of admin url uses the # to store the state, it completely override what it was supposed to do and it's not even trying to scoll to the right element. The fragment information is still available but that requires to be processed "by hand" #1129578: Overlay doesn't respect internal anchor links.
  6. toolbar + fragment: this patch is supposed to fix it #546126: Toolbar interferes with anchor links but it does not work.
  7. Now: toolbar + tableheader + overlay + fragment should be working but right now every piece of the chain is broken somehow.

I didn't really want another meta but we need to take a step back from this mess and come up with a clean solution that doesn't involves PHP.
I think #787940: Generic approach for position:fixed elements like Toolbar, tableHeader should be put back on track and simplified as much as possible, perhaps taking the fragment part to an other issue.

Anyway what is certain is that this all need to be though as a whole and a generic solution needs to be found that allow easy opt-in for contrib while not messing up the 30 years old fragment feature. There are two main issues, fragment focus in the different situations, and having a way to compute an offset for tableheader that doesn't invlove PHP, eval or monkey-patches.

If you feel this would be better as a summary in #787940: Generic approach for position:fixed elements like Toolbar, tableHeader just tell me I'll (or anyone :) update the summary there and close this. I felt like a new thread would scare people less compared to a 2+ year issue with nearly a hundred reply. If this one is closed all the other issues linked here should be closed too.

And seeing the state of this thing, that's a bug, not a feature request.

Related:

Comments

#3

tagging

#4

Less obnoxious issue summary.

#5

Oh yeah this is still very much valid.
Evertime I go to the permissions page using a direct link I get reminded of this not working, cause the scrolling does not take toolbar into account.

#6

When there is an error during form validation, an HTML5 browser will focus on the error field, and the toolbar gets in the way.

#7

nod_: I just wanted to point your attention to a possible alternative for sticky table headers: http://updates.html5rocks.com/2012/08/Stick-your-landings-position-stick...

A little internet research didn't uncover any examples that used this technique for tableheaders but it's worth a try.

#8

Indeed, someone pointed it out to me a couple weeks ago last week (time flies).

The current tableheader implementation is doing waaay less work on scroll than it used to do. And we should be using a debounce function for this kind of things anyway. Will definitely be interesting once it's supported by *cough* IE or a decent polyfill is around.

#9

nod_: that was also me. I did a search for a display: sticky polyfill and found this: http://codepen.io/FWeinb/pen/xLakC

I bet there's a better one but this seems pretty concise.

#10

Some quick analysis leads me to believe position:sticky; doesn't even try to solve the anchor problem, it seems like it's just a conditional position:fixed; with less flexibility on the positioning...

#11

I just ran in to the issues with displacement while working on a custom module so I'll start tracking this and see if there's anything I can help out with. I took a look at #787940: Generic approach for position:fixed elements like Toolbar, tableHeader and my brain melted, so maybe we do need to refocus.

#12

1, 2, 3) have been fixed by other patches.

We now have a offsettopchange event, and data-offset-top attribute used to compute the total top offset. Interesting code is in tableheader.js.

#13

New toolbar, same issues some related changes in the JS though :D

#1846970: [meta] Responsive Toolbar follow-ups

#14

nobody click here