Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve loading speed and browser compatibility #26

Merged
merged 6 commits into from
Dec 12, 2018
Merged

Improve loading speed and browser compatibility #26

merged 6 commits into from
Dec 12, 2018

Conversation

colemanw
Copy link
Collaborator

@colemanw colemanw commented Dec 9, 2018

  • Store menu data in localCache for performance
  • Cache rendered markup and add to the dom asap to prevent flash with no menu
  • Fix some race conditions when loading and rendering the menu
  • Fix quicksearch icon and width in older browsers

@colemanw colemanw mentioned this pull request Dec 9, 2018
70 tasks
@colemanw colemanw force-pushed the cache branch 2 times, most recently from b0ccb58 to 4c42740 Compare December 10, 2018 19:39
@colemanw
Copy link
Collaborator Author

@christianwach this stabilizes the height adjustment on WP and other systems. The only time the height doesn't adjust now is when the quicksearch box temporarily pushes the menu to a 2nd line, but IMO that's acceptable and maybe even desirable, as the user is focused on interacting with the quicksearch box in that moment and doesn't need to be distracted by other page content shifting around. As soon as that box loses focus the menu snaps back to its original size and all should be well.

But the main thrust of this PR is blazingly fast render speeds to avoid any jumpiness when navigating between pages. @andrewpthompson can you try this out one more time on your array of browsers and CMSs - I think we're ready to cross the finish line on this one.

@andrewpthompson
Copy link

andrewpthompson commented Dec 12, 2018

@colemanw Here are my results of re-testing of the updated #26 on Joomla 3.9.1 using Firefox 63 (Windows 10 + Fedora 28 KDE), Chrome 70 (Windows), Edge 42, IE 11.

Flash during page load - now pretty darn good:

  • Firefox – excellent, no redraw of the menu is visible during page loads.
  • Chrome is now almost as good as Firefox. On 90% of page loads there is no redraw of the menu visible at all. Just occasionally it flashes, but is now very quick. (fine)
  • IE11 - there is an extremely brief flash every time the page loads, but the Joomla menu does the same. (fine)
  • Edge – excellent, no redraw of the menu is visible during page loads.

Quick search:

  • IE11: quick search now works most of the time. About 20% of the time although the quick search box appears to have focus there is no cursor and it's not possible to type into it. After this happens, clicking it multiple times doesn't usually make the cursor appear, but one double-click or click elsewhere and click back in the quick search box again makes it work. This one is fairly trivial, maybe a double-click is actually correct for IE.
  • IE and Edge: Quick Search box expand/contract now works OK. Actually it looks like it still fails to expand on IE11 and Edge. Retested again, seems fine now.
  • Now this one has me puzzled. On my local dev server it is fine and I can't reproduce it, but on my production host (I'm just using shared hosting, so it's a different environment) for all browsers the icon is still missing in the Quick Search box. On FF & Chrome it is sometimes present briefly then replaced by "Qu" ("Quick Search") text. I think I've cleared every relevant cache etc but I can't make this one go away on my hosting. And it isn't happening without Improve loading speed and browser compatibility #26 applied. D'oh, this was due to having Shoreditch installed but not in use. OK when Shoreditch extension disabled.
    image

@andrewpthompson
Copy link

@colemanw Sorry one more trivial thing I noticed on IE11. On a Civi page that is big enough to need scrolling, the scrollbar is invisible (auto-hides) until the mouse is moved. Problem is that the scroll bar is then on top of the KAM's 'Adjust menu position' icon so it then becomes slightly awkward to click on 'Adjust menu position'. In other words, if you move the mouse to go to click on the icon the scrollbar decides to appear and gets (mostly) in your way.
2018-12-12 civicrm kam 2
I don't know why the scrollbar is auto-hiding anyway; CiviCRM doesn't do that on other browsers and IE doesn't do it on other sites that I checked.

@colemanw
Copy link
Collaborator Author

Thanks again for the testing @andrewpthompson - since we're now down to trivial stuff I'm going to merge this and release another beta so it gets more exposure. I haven't been able to reproduce that icon issue, and I'll hop back on IE11 tomorrow to see if I can reproduce the scrollbar problem. Could be Joomla-specific.

@colemanw colemanw merged commit 7374dc5 into master Dec 12, 2018
@colemanw colemanw deleted the cache branch December 12, 2018 02:28
@andrewpthompson
Copy link

@colemanw Great work :) Disregard the icon issue. That was my fault - I had Shoreditch installed, which caused it. Also at one point I thought that IE and Edge were not expanding the Quick Search box but I just re-tested more thoroughly and can't reproduce it now.

@colemanw
Copy link
Collaborator Author

@andrewpthompson yes Shoreditch will need to be updated. I've filed civicrm/org.civicrm.shoreditch#342

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants