joomlatools / joomlatools-framework

Modern PHP extension framework (for Joomla)
https://www.joomlatools.com/developer/framework/
GNU General Public License v3.0
19 stars 11 forks source link

Floatthead too slow on Firefox #58

Closed ercanozkaya closed 7 years ago

ercanozkaya commented 7 years ago

See:

If you resize the page in a crowded table, the page hangs for a few seconds.

See Docman documents view for an example with more than 50 rows.

johanjanssens commented 7 years ago

@ercanozkaya This just became a prio bug. We have another user having issues in FF.

robinpoort commented 7 years ago

@ercanozkaya This only happens on resize right? Might be good to temporarily disable footable during resize if there's no better option?

ercanozkaya commented 7 years ago

@robinpoort I traced this down to floatThead. If I remove it from admin.js the page is just fine in Firefox. Could you explain what this script does (well yeah it floats the thead) and if it can be replicated using any other techniques?

robinpoort commented 7 years ago

@ercanozkaya Not really. It was a difficult find to find one that worked good enough. We might be able to use css position: sticky but it will only have 50% support coming in january (when it's being added to Chrome). IE is a looong way of for position: sticky. It's only in consideration.

Another thing we could try is use a less good alternative that just copies the entire table head row and apply fixed positioning to it. Might not be as bad as on a website since this is a backend anyways. Here's one from CSS tricks: https://css-tricks.com/persistent-headers/

We could then add a simple aria-hidden="true" to the <tr> and tabindex="-1" to the links inside the <th>'s for a little bit better screenreader support.

Maybe you could find a solution that works fine on all latest browsers and then we can look into adding those aria and tabindex values later?

Thoughts?

johanjanssens commented 7 years ago

This is only a problem on FF. Can we serve FF a solution that works, maybe is degenerated a bit?

robinpoort commented 7 years ago

@johanjanssens I guess we could but that would require browser sniffing. What if this is fixed in the next version? We'd have to keep testing.

There's not really an alternative for this. It's either a sticky table header or not. Doesn't matter what technique we use, it'll always require JS. So why not go with a JS technique that works smoothly everywhere? Adding the aria and tabindex is easy enough for any which library/technique. There's always a point where the header becomes sticky, and also when when it becomes unsticky. We simple find the tr and set the aria-hidden and we find all tabbable elements and we set them to tabindex=-1. We already have a tabable library for the slide-out sidemenu so we can just use that.

johanjanssens commented 7 years ago

Oki, agreed.

robinpoort commented 7 years ago

@ercanozkaya Could you find a small script/library that runs smoothly on modern browsers? If you found one I can work on getting the aria-hidden and tabindex in and add it to KUI / JUI

ercanozkaya commented 7 years ago

@robinpoort I don't even know where to start since not sure what the requirements are. I also have the suspicion this might be related to our markup as it doesn't seem to happen on every floatthead installation.

Can you start with the persistent header approach from CSS tricks and see if it works good enough?

robinpoort commented 7 years ago

@ercanozkaya I think I've knocked quite some load time off of FF initial load. 100 items on a page now loads the page a little bit slower than Chrome does. Before these optimizations I got "Do you want to kill the JS on the page" warnings :)

Resising is not quite working yet though on FF. Could you take a lok at this? See what you can do about it? Left some comments in the code. You can ping me if you have any questions

robinpoort commented 7 years ago

@ercanozkaya

Replaced the lot with CSS position: sticky. Current browser compatibility is 18% but on January 31st Chrome 56 is going to be released with sticky support which brings the total percentage to about 40% / 50% within a few months. Currently position sticky is broken for Firefox table elements (https://bugzilla.mozilla.org/show_bug.cgi?id=975644) and in consideration for Edge (https://wpdev.uservoice.com/forums/257854-microsoft-edge-developer/suggestions/6263621-position-sticky). I voted for both so let's hope we'll have proper position sticky in 6-12 months. In the meantime it's working perfectly on Safari (iOs as well) in Chrome (in about a month) and also Opera at the same time (also about a month).

Can you do some tests? If you pull all extensions (master) and FW and FW-files (corresponding branches) then you can give it a spin. Also pull kodekit UI and joomlatools UI (branches) if you got further compiling to do.

robinpoort commented 7 years ago

@ercanozkaya Can you let me know when this is merged to master so I can update the developer docs? Thanks!

ercanozkaya commented 7 years ago

@robinpoort Seems fine to me. Closing the ticket, could you please merge the kodekit-ui and joomlatools-ui branches? Don't wanna override anything during recompile.

robinpoort commented 7 years ago

@ercanozkaya I merged Kodekit and joomlatools-ui to master. You did framework yourself?

ercanozkaya commented 7 years ago

@robinpoort Yes I did since there was no need for recompile there

On Mon, Jan 23, 2017 at 11:15 AM, Robin Poort notifications@github.com wrote:

@ercanozkaya https://github.com/ercanozkaya I merged Kodekit and joomlatools-ui to master. You did framework yourself?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/joomlatools/joomlatools-framework/issues/58#issuecomment-274424136, or mute the thread https://github.com/notifications/unsubscribe-auth/AAFhDh3AjJUigobI0u2ZuQQiOFZ0_NM0ks5rVGGKgaJpZM4LGf1K .

-- Ercan Özkaya