Add component menu #4414
Add component menu #4414
Conversation
|
Demo starting at https://vanilla-framework-4414.demos.haus |
|
@bartaz do you want to have a look too? |
| @@ -28,7 +28,7 @@ | |||
|
|
|||
| {% macro side_nav_item(url, title, label, type) -%} | |||
| <li class="p-side-navigation__item"> | |||
| <a class="p-side-navigation__link" href="{{ url }}" {% if path == url %}aria-current="page"{% endif %}> | |||
| <a class="p-side-navigation__link" href="{{ url }}" {% if (slug and slug in url) or (url == path) %}aria-current="page"{% endif %}> | |||
There was a problem hiding this comment.
Are both parts of this conditional needed? Would only (slug and slug in url) work or does it need the url == path for some other cases?
There was a problem hiding this comment.
Initially I had only slug in url then highlight side menu item. It was broken for the home page, where slug is "" making the condition true for all menu items, so everything was highlighted.
It's not the prettiest condition.. I agree.
|
All the pages have duplicated titles:
I'm not sure if that's something to be addressed later? Is it just a matter or removing title from markdown and keeping it in meta data only? |
templates/_layouts/docs.html
Outdated
| @@ -155,7 +155,13 @@ | |||
| </aside> | |||
|
|
|||
| <main class="col-9" id="main-content"> | |||
| {% block content %}{{ content | safe }}{% endblock content %} | |||
| {% block content %} | |||
| <h1>{{ title.split('|')[0] | safe }}</h1> | |||
There was a problem hiding this comment.
I think we can drop | safe here, we don't want any HTML code passed from title to the page (there shouldn't be any HTML there anyway), so this should be enough:
| <h1>{{ title.split('|')[0] | safe }}</h1> | |
| <h1>{{ title.split('|')[0] }}</h1> |
There was a problem hiding this comment.
Pushed a fix for the double titles problem and removed safe.
| @@ -146,7 +147,24 @@ def global_template_context(): | |||
| version_parts = VANILLA_VERSION.split(".") | |||
| version_minor = f"{version_parts[0]}.{version_parts[1]}" | |||
|
|
|||
| return {"version": VANILLA_VERSION, "versionMinor": version_minor, "path": flask.request.path} | |||
| docs_slug = flask.request.path.replace("/docs/", "") | |||
| docs_slug = docs_slug.replace("/design/", "") | |||
There was a problem hiding this comment.
I'm sure there's a good reason not to... but why not chaining the replace method instead of reassigning the variable several times?
There was a problem hiding this comment.
The max 80 char per line rule.
There was a problem hiding this comment.
I think something like this would do the trick: (It's not particularly pretty, though... Feel free to leave as it is)
docs_slug = (
flask.request.path.replace("/docs/", "")
.replace("/design/", "")
.replace("/accessibility", "")
)There was a problem hiding this comment.
Good suggestion! I will try it.
albertkol
force-pushed
the
add-component-menu
branch
from
fd80899
to
41d4eb1
Compare
albertkol
force-pushed
the
new-vanilla-website
branch
from
44fddb0
to
72bfb66
Compare
albertkol
added
Review: QA +1
Review: Code +1
and removed
Review: QA needed
Review: Code needed
labels
Done
Add
component_tabs.yamlfile that stores the /docs tab menu. It will be the source of truth for when the menu shows up on each docs page and which tab items get displayed (design/accessibility).Structure of the
component_tabs.yaml:This PR add the menu only for the accordion docs page, future pages will be added in a separate PR.
QA
Screenshots