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

Bad display of sidebar caused by table of content widget #926

Closed
CrSjimo opened this issue Sep 5, 2021 · 2 comments
Closed

Bad display of sidebar caused by table of content widget #926

CrSjimo opened this issue Sep 5, 2021 · 2 comments
Labels
bug:core Issues about layout, styles, scripts, etc. bug:extension Issues about widgets, comment, share, search, and plugins.

Comments

@CrSjimo
Copy link
Contributor

CrSjimo commented Sep 5, 2021

Describe the bug
Table of contents is the only widget included in the right sidebar. When displaying a page without table of contents (e.g. homepage), it layouts as if the right sidebar was still there (actually nothing is shown).

System and Environment

  • hexo 5.0.2
  • icarus 4.4.0
  • Firefox 92.0

Screenshots
Screenshot

Additional context
The problem is probably caused by the function hasColumn at widgets.jsx:22. The function detects whether a sidebar has any widgets by traversing the configuration to check if the position is configured to any widgets. However, it does not consider whether table of contents widgets displays on current page or not, and it retures a wrong existence of widget.

@CrSjimo
Copy link
Contributor Author

CrSjimo commented Sep 5, 2021

I have tried to fix the problem
If I detect whether toc exists in the function mentioned above, the bug will be fixed:

function hasColumn(widgets, position, config, page) {
    const showToc = (config.toc === true || page.toc) && ['page', 'post'].includes(page.layout);
    if (Array.isArray(widgets)) {
        return typeof widgets.find(widget => {
            if(widget.type === 'toc' && !showToc)return false;
            return widget.position === position
        }) !== 'undefined';
    }
    return false;
}

But the side effect of this change is that the argument config and page should be passed to this function at every call of the function. I attempted to test it but I do not know how to test such a project. Anyhow, I indeed changed every reference of the function (in widgets.jsx and layout.jsx), adding the two arguments in the parameter list of every call and it seems work.

May I file a pr? ('・ω・')

@ppoffice
Copy link
Owner

ppoffice commented Sep 6, 2021

@CrSjimo Yes, please go ahead with the PR.

Some other suggestions:

  1. You can omit checking page.toc since this config is merged into config variable. An example is here.
  2. Please keep all semicolons and brackets.

@ppoffice ppoffice added bug:extension Issues about widgets, comment, share, search, and plugins. bug:core Issues about layout, styles, scripts, etc. labels Sep 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug:core Issues about layout, styles, scripts, etc. bug:extension Issues about widgets, comment, share, search, and plugins.
Projects
None yet
Development

No branches or pull requests

2 participants