Isis fix for safari bug #5970

[dgrammatiko]

This PR drops the current custom code for sticking the two top toolbars together and instead use the already loaded bootstrap script!

Should work with all browsers but the idea was to fix #5970 and safari!

@wilsonge If this gets merged you got a nice conflict with 3.5-dev ???? Sorry!

@losedk @JoomliC and anyone with safari ever experienced this glitch can you give this a test?

[peterlose]

Seems to fix the problem :)

However noticed two things. The affix kicks in immediately on scroll and when clicking on menu items in the sidebar menu it jumps a bit

[dgrammatiko]

@losedk So I fixed a bug by introducing another one? 😃
For me the instant effect is not a problem and the jump of the sidebar is really unnoticeable...

[peterlose]

Hehe 😄 Wouldn't call them bugs, just minor issues. But let's get some people to test :)

[dgrammatiko]

@losedk actually this is a problem caused by safari’s rubber band, all other browsers are totally fine (with or without this PR). But also I think is better to use already loaded scripts (bootstrap) instead of writing our own custom ones.
And also with this PR you will never end up with a messed up screen

[peterlose]

@dgt41 Ahh okay, I didn't know that. I totally agree that it's much better to use the native BS.

[dgrammatiko]

@infograf768 JM can you test this with com_localize as I am touching the sidebar script here and AFAIK there were some compatibility issues when sidebar was introduced. Thanks

[infograf768]

@dgt41
Will do asap.

[infograf768]

@dgt41
Looks like it's fine in com_localise.

[zero-24]

RTC based on Tests by @losedk and @infograf768 Thanks 😄

---

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/7185.}

[dgrammatiko]

I just found another less intruding way to solve this:

html {
height: 100%;
}

That will not require a change for our scripts here

[wilsonge]

From a technical perspective I'd imagine it's better to use the bootstrap affix though than hack it ourselves and then add in fixes when the functionality already exists? I mean I'd rather get it right than bug fix the hacks :P

[dgrammatiko]

Apart from the wrong decision to affix also the sidebar (that’s a big change from the previous behavior and also is buggy), I totally agree with you.

[infograf768]

@test
Still OK here for core and com_localise.

[brianpeat]

Seems to fix the issue for me in 2 sites once I remembered to clear my cache.

---

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/7185.}

[brianteeman]

Multiple good tests setting to RTC

@wilsonge would be good to get this into 3.4.2

---

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/7185.}

[dgrammatiko]

@rdeutz Robert are there any particular reasons to postpone this for 3.4.3?

[Bakual]

We only fix critical things during an RC. As far as I understood this is only a minor visual thing.

[brianpeat]

I would not consider this minor. There are times when I am completely unable to get to buttons because the header never expands. It's enough to make me almost want to stop using Safari because at times the admin screens are unusable (can't name something, can't access the New button, etc).

[dgrammatiko]

@Bakual Fair enough, I just guessed since we have this fix that annoy a lot of people maybe we could also include that as well.

[mbabker]

@dgt41 IMO this is a step backwards as now we again have the jumping subnav bar.

[dgrammatiko]

@mbabker jumping? You mean the animation is not as smooth as it was or you get some other glitch?

[mbabker]

No animation at all, just jumps... http://cl.ly/0s0v0i0e0S17

[dgrammatiko]

I do not quite agree with @mbabker that this is a step backwards, but I also realize that a better animation can be applied here. So with the latest commit I try to follow the same route Joomla did with sidebar: use css3 transition.

I would like to ask if you agree with this animation or if it is too long/short?
So @losedk @mbabker @brianpeat @infograf768 your feedback here is essential, Thanks

[mbabker]

The animation is only triggering for me in Safari; Chrome and Firefox still just jump.

So here's why I say it's a step backwards. Right now, for the most part, the toolbar smoothly scrolls with the page and affixes when it reaches the target point. With this PR applied, scrolling one pixel causes it to jump up to its affixed spot. So in fixing an issue for one case, we're introducing a less than optimal behavior for another case.

FWIW, this same JavaScript logic is in use on the Bootstrap based joomla.org template and there I can't duplicate the behavior in Safari of the page content not being below the second toolbar (in the .org case it's the site menu). Actually, when the page loads "properly", unpatched I see the body contents get pushed down during rendering in the Joomla admin, but the .org sites I tested against (API which is using pure HTML and an unaltered Bootstrap source and Developer which is a CMS install) don't exhibit the same behaviors.

[dgrammatiko]

The problem is that the current script used in Isis is somehow conflicting with the bootstrap collapse for the same toolbar. If you remove the collapse from the subhead everything works nicely. I think we have many competing scripts (sidebar, collapse, affix) and somehow there is an interference as all of them trying at the same time to calculate the save values e.g. height, width

[mbabker]

Without redesigning the whole top section I don't see there being a good fix for this whole thing. Either something has to stop being affixed or we have to deal with rather ugly behaviors on scroll.

[dgrammatiko]

@mbabker I will agree on that redesign! Anyways I gave it one more try fixing FF,chrome,opera,IE11 animation

[dgrammatiko]

@mbabker messing around with the old script I found a root of the problem, so indeed we don’t have to drop the old script. Here it is:

            function processScroll()
            {
                if ($('.subhead').length) {
                    var scrollTop = $(window).scrollTop();
                    if (scrollTop >= navTop && !isFixed) {
                        isFixed = true;
                        $('.subhead').addClass('subhead-fixed');

                        // Fix the container top
                        $(".container-main").css("top", $('.subhead').offset().top + $('nav.navbar').height());
                    } else if (scrollTop <= navTop && isFixed) {
                        isFixed = false;
                        $('.subhead').removeClass('subhead-fixed');
                    }
                }
            }

[dgrammatiko]

OK I am gonna leave the decision to PLT if they want to merge this or #7342 which solves the problem with the old script 😃

[cyrezdev]

It is not having the expected behavior for me (just tested on FF), and with issue on mobile device (the last pixel up render smaller toolbar buttons)...
In the same time, #7342 is working nice, and don't change the behavior (i understand your goal with this PR, but not sure it's an easy job to introduce it in Isis (and not yet working with fluidity), or to keep some new stuff for a future new admin template ;-) )

---

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/7185.}

[wilsonge]

As this still seems buggy for people and as metioned #7342 is working ok. going to close this for #7342 - because at the end of the day we need something that works.