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

Use function 'getListItems()' to build the menu list items #43

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

lpaulsen93
Copy link

This fixes the not working "fold/unfold all" button of the folded plugin. See dokufreaks/plugin-folded#68.

I assume this might break some things. Not everything looks like before but some part of the menu look less broken. But it fixes the issue with the folded plugin and maybe (not tested) issues with other plugins which are triggered by the menu.

The issue with the folded plugin was caused because the a element did not have all attributes properly set. But it works with the standard DokuWiki template which calls getListItems() to build the menu and that is the reason why I used that function call in this PR.

This fixes the not working "fold/unfold all" button of the folded plugin.
@SoarinFerret
Copy link
Contributor

Doing some quick checks, it looks like it shouldn't be too difficult to modify this so we can keep the Argon styling we had before, while maintaining the benefits of using the getListItems() method.

For context, here is the current view:
image

And here is the updated view with this PR:
image

Looking at the differences in the HTML for the Page Menu:

Before

<li class="edit">
    <a class="page-menu__link edit" href="/doku.php?id=start&amp;do=edit" title="Edit this page" accesskey="e">
        <i class="">
            <svg xmlns="http://www.w3.org/2000/svg" width="24" height="24" viewBox="0 0 24 24">
                <path d="M20.71 7.04c.39-.39.39-1.04 0-1.41l-2.34-2.34c-.37-.39-1.02-.39-1.41 0l-1.84 1.83 3.75 3.75M3 17.25V21h3.75L17.81 9.93l-3.75-3.75L3 17.25z"></path>
            </svg>
        </i>
        <span class="a11y">Edit this page</span>
    </a>
</li>

After

<li class="edit">
    <a href="/doku.php?id=start&amp;do=edit" title="Edit this page [e]" rel="nofollow" accesskey="e">
            <span>Edit this page</span>
            <svg xmlns="http://www.w3.org/2000/svg" width="24" height="24" viewBox="0 0 24 24">
                <path d="M20.71 7.04c.39-.39.39-1.04 0-1.41l-2.34-2.34c-.37-.39-1.02-.39-1.41 0l-1.84 1.83 3.75 3.75M3 17.25V21h3.75L17.81 9.93l-3.75-3.75L3 17.25z"></path>
            </svg>
    </a>
</li>

The biggest differences I see:

  • The class="a11y" being applied to the <span> tag, which appears to just move the text completely off screen at -99999em - probably just best to not include the span or mark it as hidden?
  • The class="page-menu__link edit" being applied to the <a> tag - which doesn't appear to provide from a CSS side of things.
  • The <i> tag enclosing the logo. Also looks like this could be removed.
  • The rel="nofollow" not being added to the <a> tag. This seems to be something Dokuwiki is adding, so probably would want that.

Essentially, looking at the Page menu, we want the <span> tags removed from the output. I wonder if we could just do a replace in PHP before echoing the output for an easy fix?

As for the Site Tools menu, looks like we just want to remove the SVG next to the text to keep it looking the same.

And finally for the User Menu, we would again remove the <span> tag, and re-add the class that gave the icons the white color.

Thoughts?

- added classes 'argon-doku-navbar-icon' and 'nav-link' to the user menu
- do not show text spans in the user and page menu
- do not show SVG in the site menu
@lpaulsen93
Copy link
Author

Ok, I now applied some simple changes:

  • not showing the mentioned spans and SVGs seemed to be the simplest solution so I removed them via CSS display none
  • I added the classes argon-doku-navbar-icon and nav-link by passing them to the getListItems() call

The user menu doesn't exactly look the same as before. It would be really cool if the CSS could be adjusted to the code in this PR because the changes for now are really simple. The difference is that getListItems() sets the classes on the li element.

@SoarinFerret
Copy link
Contributor

@lpaulsen93 you mentioned it still doesn't look exactly the same, could you provide some screenshots of what looks different? I haven't had time to take a look at this yet. Maybe we can work some additional CSS magic to correct it.

@lpaulsen93
Copy link
Author

Sorry for the late reply. Here are the pictures before the change (the original):
ArgonSiteToolsBefore

ArgonTopMenusBefore

And after the changes (with this pull-request applied):
ArgonSiteToolsAfter

ArgonTopMenusAfter

It looks quite good but the spacing between the icons on the blue bar is different. And I just noticed that my TOC is gone/not displayed for some reason.

@Klap-in
Copy link

Klap-in commented Feb 7, 2021

I guess this would solve this issue as well: splitbrain/dokuwiki-plugin-dw2pdf#420

@traviswaelbro
Copy link

Looks like this would fix #35

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.

4 participants