Fix reported scrollbar issues #4632

ghost · 2019-09-22T18:07:41Z

Tested on two locations

CMS web page issue

This Pull request relates to github issue: Mousewheel scrolling error in cms page #4621

See results:

(Tested on Chrome V79 and Firefox 71)

Notes: The issue was the position using absolute and not sticky, which caused a forever scrolling action. Position: sticky has fixed this issue.

Media manager web page issue

Also tested this pull request with regards to github issue: Flickering on Media manager files after latest update #4601

See results:

(Tested on Chrome V79 and Firefox 71)

Notes: The issue here was because the scrollbar javascript in the media manager needs to have the right sidebar displayed and the css was using display: none, switching it over to opacity: 0 allows the javascript to scroll the container. If you set the display to none you will see the data-visible attribute keep trying to set it to true and the screen starts flickering! The right sidebar in the media manager is the place where the media file previews are displayed.

Conclusion

I have done three main tests here:

CMS forever scrolling issue - passed.
Media manager scrolling issue displaying the preview right hand sidebar - passed.
Media manager scrolling issue hiding the preview right hand sidebar - passed.

bennothommo · 2019-09-23T07:45:44Z

@ayumihamasaki2019 As per @Samuell1's comment, we can't use sticky as it is not supported by a fair few browsers we are still targeting.

w20k · 2019-09-23T08:10:08Z

Pretty much IE 11 is not in the list, yet.

ghost · 2019-09-23T10:17:56Z

@bennothommo @w20k @LukeTowers @daftspunk

Thanks for your comments, looking at caniuse: https://www.caniuse.com/#feat=mdn-css_properties_position_sticky

Only IE11 seems to be the issue, so let's polyfill position: sticky and get this issue sorted out once and for all.

Going to add a polyfill into the october.min.js code section, please review my next commit, ~~I will work on this and submit it within the hour~~.

Let me know your thoughts and any adjustments.

p.s. The polyfill will use ES5 to allow old browsers and support for IE9 - IE11 etc has been tested.

p.s. IE11 end of service life is on January 14, 2020.

bennothommo · 2019-09-24T04:14:09Z

@ayumihamasaki2019 Just to note, January 14, 2020 is the EOL for IE 10, not IE 11. Internet Explorer 11 is being supported for the lifecycle of Windows 7 SP1, Windows 8.1 and Windows 10. See here for more info: https://support.microsoft.com/en-us/help/17454/lifecycle-faq-internet-explorer

Given this, we still have to support IE 11 for now.

w20k · 2019-09-24T08:08:15Z

If think IE11 issues could fixed with a simple position: fixed; bottom: 0, but needs testing.

ghost · 2019-09-24T12:04:11Z

@bennothommo thanks for the link

@w20k I tested your suggestion no good, see here:

Above: using position: fixed; bottom: 0

Samuell1 · 2019-09-24T12:40:06Z

Simple fix will to change height: 100%; to something else because thats why it does that infinite scroll.

ghost · 2019-09-24T12:41:45Z

@Samuell1

Simple fix will to change height: 100%; to something else because thats why it does that infinite scroll.

Doesn't work!

Samuell1 · 2019-09-24T12:42:01Z

For me it works when i change it to 100vh :D

And i think this is issue because parent element is too 100% height.

(Looking trough it everything is table, that can be issue why is everything height 100% 😄 )

ghost · 2019-09-24T12:44:21Z

@bennothommo @w20k @LukeTowers

Ok I have finished the update. What I have done is the following solution:

All modern browsers will use: position: sticky

Internet Explorer uses a hack and will use: position: absolute

Have tested it and all working in IE11, Safari, Opera, Chrome V79, Firefox V71

ghost · 2019-09-24T12:57:22Z

Proof

See below gif's as proof, both CMS and Media Manager working fine with this Pull Request.

Internet Explorer (IE11)

Firefox V71

Chrome V79

etc.

ghost · 2019-09-25T13:00:45Z

@w20k @bennothommo @Samuell1 @LukeTowers

If anyone got any free time, I would be greatful if you could test this pull request and add the label testing now. As seen in the above comment everything is fixed and working.

Thanks in advance.

roojai · 2019-09-28T05:48:21Z

I have tested these changes in Chrome 77 and I still get the same problem.

Using Canary (Chrome 79) the media manager works perfectly well with or without these changes.

ghost · 2019-09-28T07:45:25Z

I'm going to kill two birds with one stone in this comment ...

Part 1

@roojai

I have tested these changes in Chrome 77 and I still get the same problem.

Using Canary (Chrome 79) the media manager works perfectly well with or without these changes.

Let me guess you only tested Google Chrome and not any other browser! This Pull Request fixes cross browser issues - see my below comment with a Firefox example.

Part 2

@w20k

As per this conversation:

I feel you're not listening to what I'm trying to tell you - to help you out!

Please look at the bottom Firefox example demoing the data-visible attribute issue.

Without this PR (In Firefox V71)

You will see with preview it works! Without preview it doesn't work!

With this PR (In Firefox V71)

You will see with preview it works! Without preview it works!

What did I do?

The issue in the media manager is the data-visible attribute. I never touched the position:sticky in the media manager - I touched that in the CMS web page!

Adding the following to the media manager section fixes the data-visible attribute issue:

Conclusion

Therefore the fix is very simple to understand, add the code I showed in the above picture and also restrict the canvasSize limit via javascript (for the cms web page issue) will fix both issues at the same time!

Hopefully now you can understand what I'm talking about.

roojai · 2019-09-28T09:04:31Z

@ayumihamasaki2019

My mistake - I wrongly assumed this was a fix for the issue with the current stable release of Chrome.

Sorry, and appreciate your work.

ghost · 2019-09-28T10:31:03Z

@roojai no worries, dont worry about it. If you come across any issues with the scrollbar let me know and I will be happy to add any extra fixes to this pr.

w20k · 2019-10-26T13:27:31Z

Should be fixed in the latest chrome build v78.

summercms · 2019-12-07T14:16:24Z

@w20k

In this comment: #4632 (comment)

I demo the error in Firefox V71 and you write:

"Should be fixed in the latest chrome build v78."

What the heck! You can't do like that.

LukeTowers · 2019-12-07T20:00:11Z

@ayumi-cloud please submit another issue if this is still a problem. Given that the original account was deleted I'd prefer this issue was started over.

w20k · 2019-12-07T21:02:05Z

@ayumi-cloud please could you try to re-read the comments from all the tickets related to this flickering, before posting angry comments?
Second, to add more to your comments, I think it's also @LukeTowers who said it once: "closing doesn't mean `anything. It's our way to organize issues/PRs and our workflow".
And @LukeTowers comment above ^.

Nor I, nor @bennothommo could not reproduce the same behavior you had with Firefox. I've tried it with the latest and canary (Dev Nightly). The only thing I had and knew how to reproduce was flickering in MediaManager (#4601) and no one reported issues related to FF if I recall correctly.

This occurs due to a race condition in the rendering where the scrollbars enable and disable over and over because of a slow height calculation. Giving any height number appears to close the loop by never letting the height resolve to 0 Refs #4632

summercms · 2019-12-07T23:31:02Z

@LukeTowers @w20k

Actually, I'm speaking with daftspunk in private and we both can reproduce this error! See video of the error here: https://youtu.be/Rlq9R3WIDfQ

Commit to fix it: 1e449c8

LukeTowers · 2019-12-10T18:45:40Z

Awesome, thanks for following up with this @ayumi-cloud!

Fix CMS scrollbar issue

847ef4b

ghost changed the title ~~Fix CMS scrollbar issue~~ Fix reported scrollbar issues Sep 22, 2019

w20k added Status: Review Needed Type: Maintenance labels Sep 22, 2019

Ayumi Hamasaki added 2 commits September 22, 2019 20:57

Fix the media manager issue

9f4d06c

future ref

8872d04

ghost mentioned this pull request Sep 22, 2019

Flickering on Media manager files after latest update #4601

Closed

Update mediamanager.css

0abc0a3

This was referenced Sep 22, 2019

Mousewheel scrolling error in cms page #4621

Closed

MediaFinder - List 'Render' Glitch #4630

Closed

bennothommo added Status: Revision Needed and removed Status: Review Needed labels Sep 23, 2019

Ayumi Hamasaki added 2 commits September 23, 2019 11:31

Added position sticky polyfill for IE11 support

a8be695

Little tidy up

2ea424c

Updated Internet Explorer fix

e72d40d

typo

f689771

w20k added the Testing Needed label Sep 25, 2019

LukeTowers added this to the v1.0.460 milestone Sep 25, 2019

LukeTowers requested a review from daftspunk September 25, 2019 15:49

ghost mentioned this pull request Oct 16, 2019

Improve responsiveness of backend elements #4694

Closed

w20k closed this Oct 26, 2019

LukeTowers added Status: Completed and removed Status: Revision Needed Testing Needed labels Dec 10, 2019

LukeTowers modified the milestones: v1.0.460, v1.0.461 Dec 10, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix reported scrollbar issues #4632

Fix reported scrollbar issues #4632

ghost commented Sep 22, 2019 •

edited by ghost

Loading

bennothommo commented Sep 23, 2019

w20k commented Sep 23, 2019

ghost commented Sep 23, 2019 •

edited by ghost

Loading

bennothommo commented Sep 24, 2019 •

edited

Loading

w20k commented Sep 24, 2019

ghost commented Sep 24, 2019

Samuell1 commented Sep 24, 2019

ghost commented Sep 24, 2019

Samuell1 commented Sep 24, 2019 •

edited

Loading

ghost commented Sep 24, 2019 •

edited by ghost

Loading

ghost commented Sep 24, 2019 •

edited by ghost

Loading

ghost commented Sep 25, 2019

roojai commented Sep 28, 2019

ghost commented Sep 28, 2019 •

edited by ghost

Loading

roojai commented Sep 28, 2019

ghost commented Sep 28, 2019

w20k commented Oct 26, 2019

summercms commented Dec 7, 2019

LukeTowers commented Dec 7, 2019

w20k commented Dec 7, 2019

summercms commented Dec 7, 2019

LukeTowers commented Dec 10, 2019

Fix reported scrollbar issues #4632

Fix reported scrollbar issues #4632

Conversation

ghost commented Sep 22, 2019 • edited by ghost Loading

Tested on two locations

CMS web page issue

Media manager web page issue

Conclusion

bennothommo commented Sep 23, 2019

w20k commented Sep 23, 2019

ghost commented Sep 23, 2019 • edited by ghost Loading

bennothommo commented Sep 24, 2019 • edited Loading

w20k commented Sep 24, 2019

ghost commented Sep 24, 2019

Samuell1 commented Sep 24, 2019

ghost commented Sep 24, 2019

Samuell1 commented Sep 24, 2019 • edited Loading

(Looking trough it everything is table, that can be issue why is everything height 100% 😄 )

ghost commented Sep 24, 2019 • edited by ghost Loading

ghost commented Sep 24, 2019 • edited by ghost Loading

Proof

Internet Explorer (IE11)

Firefox V71

Chrome V79

ghost commented Sep 25, 2019

roojai commented Sep 28, 2019

ghost commented Sep 28, 2019 • edited by ghost Loading

Part 1

Part 2

Without this PR (In Firefox V71)

With this PR (In Firefox V71)

What did I do?

Conclusion

roojai commented Sep 28, 2019

ghost commented Sep 28, 2019

w20k commented Oct 26, 2019

summercms commented Dec 7, 2019

LukeTowers commented Dec 7, 2019

w20k commented Dec 7, 2019

summercms commented Dec 7, 2019

LukeTowers commented Dec 10, 2019

ghost commented Sep 22, 2019 •

edited by ghost

Loading

ghost commented Sep 23, 2019 •

edited by ghost

Loading

bennothommo commented Sep 24, 2019 •

edited

Loading

Samuell1 commented Sep 24, 2019 •

edited

Loading

ghost commented Sep 24, 2019 •

edited by ghost

Loading

ghost commented Sep 24, 2019 •

edited by ghost

Loading

ghost commented Sep 28, 2019 •

edited by ghost

Loading