ppoffice / hexo-theme-icarus

A simple, delicate, and modern theme for the static site generator Hexo.
https://ppoffice.github.io/hexo-theme-icarus/
MIT License
6.28k stars 1.53k forks source link

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

Closed CrSjimo closed 2 years ago

CrSjimo commented 2 years ago

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

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 commented 2 years ago

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 commented 2 years ago

@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.