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

Cumulative Layout Shift (CLS) is high on mobile device. #3351

Open
AmritasyaPutra opened this issue Nov 27, 2020 · 8 comments
Open

Cumulative Layout Shift (CLS) is high on mobile device. #3351

AmritasyaPutra opened this issue Nov 27, 2020 · 8 comments

Comments

@AmritasyaPutra
Copy link
Contributor

Website: https://wiki.ekvastra.in/doku.php
Running 2020-07-29 "Hogfather" with default skin.

On mobile the sidebar converts into a collapsed menu on the top.

Google gives a low score on core web vitals because of Cumulative Layout Shift (CLS) https://web.dev/cls/

The menu starts fully expanded and collapses in a split second, this causes an annoying change in layout. Is there a way to make the menu start in collapsed state so that the layout does not change as the page loads? This may not be reproduced if you do not have a sidebar or a sidebar with only one item, please check this from your mobile on the site I provided.

@Klap-in
Copy link
Collaborator

Klap-in commented Nov 27, 2020

This is a combination of css and JavaScript which are getting the desired state open/closed menu in more steps.

#1496 (comment) (Discussion is bit different than actual thread topic)

https://bugs.dokuwiki.org/2926.html Older, more complete description of the issue.

It is a bit elaborated to improve because the JavaScript is handled and set at more places. Partly due to the no-javascript fallback.

@AmritasyaPutra
Copy link
Contributor Author

Is there a simple change by which I can completely forget the sidebar/dropmenu on mobile device. I would be okay to do that for my site for mobile devices.

@Klap-in
Copy link
Collaborator

Klap-in commented Nov 28, 2020

No, then it would be done. We have divided the code over some parts: the core and the templates, that bothers a quick fix. Most quick fixes I have seen move the CLS to another moment/form factor.

@stapelberg
Copy link
Contributor

BTW, #3347 will help with CLS for images, in case that affects your site, too.

@AmritasyaPutra
Copy link
Contributor Author

Is there a simple css change that I can add from main.php such that dokuwiki__aside is hidden by default and the javacript load can expand it? That way the CLS problem moves from the mobile to the desktop. I want to prioritize my mobile users.

@AmritasyaPutra
Copy link
Contributor Author

AmritasyaPutra commented Jun 17, 2021

I made the following CSS changes:
h3 class="toggle closed" style="cursor: pointer; display: block;"
div class="content" style="display: none;" aria-expanded="false"><div class="group" style="display: none;"

... so now it starts collapsed on mobile... then there is some JavaScript that expands it... and then there is some more Javascript that collapses it again! IMHO this is not good behavior, there should be one CSS and one JS behavior around this... the first run of the JS that expands it without looking at the device property is not needed and feels like a bug. Without Javascript it remains collapsed and cannot be expanded, I am okay with this lost functionality for users that do not have Javascript enabled. What I want to do is make it mobile first at the cost of desktop users. Even with CSS changes there is at least two more JS calls, the first one blindly expands it and the second one decides whether it should remain expanded or get collapsed based on the device/screen size.

@AmritasyaPutra
Copy link
Contributor Author

AmritasyaPutra commented Jun 17, 2021

I added -1 as last parameter of makeToggle call in page.js, and script.js ... now it starts and stays collapsed in mobile until I click on it, so no Layout shift. On desktop it starts collapsed and then expands, which is acceptable to me. In fact it does not cause Layout "shift" in desktop either because the sidebar area was empty and no text had to be pushed out to expand the sidebar. This is a win-win on both mobile and desktop as far as Layout shift is concerned. We might as well remove the animation and make the experience much superior. Please comment what can be the simplest way to achieve this and if an option can be added to make the site mobile first or desktop first which can control this.

@bleistivt
Copy link
Contributor

@AmritasyaPutra I solved this by monkey-patching dw_page.makeToggle in my theme. Example for #dw__toc:

var originalMakeToggle = dw_page.makeToggle;
dw_page.makeToggle = function(handle, content, state){
    if (handle === '#dw__toc h3' && content === '#dw__toc > div') {
        var mobile = jQuery('#screen__mode').css('z-index') + '';
        originalMakeToggle(handle, content, (mobile === '1' || mobile === '2') ? -1 : 1);
    } else {
        originalMakeToggle(handle, content, state);
    }
};

This can be used together with a CSS rule like you proposed above:

@media (max-width:__tablet_width__){
    /* Hide TOC on load */
    #dw__toc > div, #dw__toc > div > ul.toc{display:none;}
}

...which could then be overridden in a <noscript> block if you want to serve non-JS users as well.

The main problem seems to be that the mobile detection is a function of the template and should probably be moved to the general JS utils so the makeToggle calls can receive the correct state in the third parameter.

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

No branches or pull requests

4 participants