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

Navigation menu plus plugin hook #1064

Closed
simonw opened this issue Oct 30, 2020 · 10 comments · Fixed by #1065
Closed

Navigation menu plus plugin hook #1064

simonw opened this issue Oct 30, 2020 · 10 comments · Fixed by #1065

Comments

@simonw
Copy link
Owner

simonw commented Oct 30, 2020

Needed for #690. Prototype in 0d7ac76

@simonw simonw added this to the 0.51 milestone Oct 30, 2020
@simonw
Copy link
Owner Author

simonw commented Oct 30, 2020

Here's what the prototype looks like so far:

menu

@simonw
Copy link
Owner Author

simonw commented Oct 30, 2020

I used a <details><summary> for this:

<header><nav>{% block nav %}
<details class="nav-menu">
<summary><svg
fill="currentColor" stroke="currentColor" xmlns="http://www.w3.org/2000/svg"
viewBox="0 0 16 16" width="16" height="16">
<path fill-rule="evenodd" d="M1 2.75A.75.75 0 011.75 2h12.5a.75.75 0 110 1.5H1.75A.75.75 0 011 2.75zm0 5A.75.75 0 011.75 7h12.5a.75.75 0 110 1.5H1.75A.75.75 0 011 7.75zM1.75 12a.75.75 0 100 1.5h12.5a.75.75 0 100-1.5H1.75z"></path>
</svg></summary>
<div class="nav-menu-inner">
<ul>
<li><a href="/">Home</a></li>
<li><a href="/-/plugins">Installed plugins</a></li>
<li><a href="/-/versions">Software versions</a></li>
<li><a href="/-/metadata">Metadata</a></li>
{% if show_logout %}
<form action="{{ urls.logout() }}" method="post">
<input type="hidden" name="csrftoken" value="{{ csrftoken() }}">
<button class="button-as-link">Log out</button>
</form>{% endif %}
</ul>
</div>
</details>

I added a bit of JavaScript so that clicking outside the menu would close it:

<script>
var menuDetails = document.querySelector('.nav-menu');
document.body.addEventListener('click', (ev) => {
/* was this click outside the menu? */
if (menuDetails.getAttribute('open') !== "") {
return;
}
var target = ev.target;
while (target && target != menuDetails) {
target = target.parentNode;
}
if (!target) {
menuDetails.removeAttribute('open');
}
});
</script>

@simonw
Copy link
Owner Author

simonw commented Oct 30, 2020

Tips for making this accessible: https://css-tricks.com/accessible-svgs/

@simonw
Copy link
Owner Author

simonw commented Oct 30, 2020

So what should go in this menu?

If the user is logged in as root, I'll link to the various debug pages.

If they're not logged in at all I don't think the menu should appear.

If they are logged in as anyone, it should display to give them access to the "log out" button.

Plugins can add links to it. If those plugins add links, the menu will display.

@simonw simonw changed the title Navigation menu Navigation menu plus plugin hook Oct 30, 2020
@simonw
Copy link
Owner Author

simonw commented Oct 30, 2020

How should the plugin hook work?

Here's the first version of the HTML:

        <div class="nav-menu-inner">
            <ul>
                <li><a href="{{ urls.instance() }}">Home</a></li>
                <li><a href="{{ urls.path('/-/plugins') }}">Installed plugins</a></li>
                <li><a href="{{ urls.path('/-/versions') }}">Software versions</a></li>
                <li><a href="{{ urls.path('/-/metadata') }}">Metadata</a></li>
                {% if show_logout %}
                <form action="{{ urls.logout() }}" method="post">
                    <input type="hidden" name="csrftoken" value="{{ csrftoken() }}">
                    <button class="button-as-link">Log out</button>
                </form>{% endif %}
            </ul>
        </div>

@simonw
Copy link
Owner Author

simonw commented Oct 30, 2020

Should plugins be able to add forms like the logout form here, or should they be restricted to adding navigation links?

I can't think of a reason a plugin would need to add a form. The logout form is a special case to protect against logout-csrf attacks.

So I think plugins get to return a list of dictionaries, each with a label and an href:

return [{
  "label": "Upload CSVs",
  "href": datasette.urls.path("/-/upload-csvs")
}]

But... is there an argument for returning headings, to divide up the menu?

I think so. I also like the idea that a default plugin checks for the root user and outputs links to the different debugging tools - maybe those should be wrapped in a section heading.

@simonw
Copy link
Owner Author

simonw commented Oct 30, 2020

Or... plugins could return HTML - maybe optionally using helper functions to generate common HTML such that plugins which use the helpers can have their HTML modified in the future.

@simonw
Copy link
Owner Author

simonw commented Oct 30, 2020

Plugins returning HTML makes more sense for some of the other areas that plugins will be able to inject content - e.g. injecting content on the table or row page above the table.

If I'm going to have that as a pattern though it may make sense to use HTML here, since that will be consistent with other places that plugins can inject additional content.

@simonw
Copy link
Owner Author

simonw commented Oct 30, 2020

I'm torn on this one. I think I have a very slight preference for plugins returning structured objects as opposed to HTML. Less likely to regret that choice in the future?

@simonw
Copy link
Owner Author

simonw commented Oct 30, 2020

I'm going to go with a list of {"label": ..., "href": ...} as the first iteration of this. The logout link will not be returned as part of the plugin output. A default plugin will provide the debug tools if the user is logged in as root.

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

Successfully merging a pull request may close this issue.

1 participant