Add support for multiple tables of contents in the Solidus Guides #3093
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
As we gain multiple guides (e.g. Developers Guide, User Guide, API Guide), the /index.html should be reserved as a landing page. Instead of using /index.html while the Solidus Guides are only the Solidus Developers Guide, let's redirect the homepage to /developers/index.html.
houndci-bot
reviewed
Feb 11, 2019
|
@benjaminwil if you want to take a look it would be awesome, thanks! |
This commit adds helper methods that will make rendering different tables of contents on different parts of the site easy. This also updates the discover_title helper to account for multiple tables of contents. \## Dealing with multiple tables of contents For example, if I visit the following URL: `localhost:4567/developers/orders/overview.html` I should see the table of contents for the "Solidus Developers Guide". And if I visit this URL: `localhost:4567/users/promotions/overview.html` I should see the table of contents for the "Solidus Users Guide". Here's a description of the helpers's functionality: \## table_of_contents The table_of_contents helper allows you to determine which table of contents should be rendered. If the user is viewing a URL with the subdirectory `/developers/`, then the current table of contents will be at the `/data/nav/developers.yml` YAML file. \## current_guide the current_guide helper finds which guide the user is viewing. If they are a URL with the subdirectory `/developers/`, then the current guide is the "Solidus Developers Guide". The current_guide returns the YAML at `/data/nav/global.yml` which allows you to get the `current_guide.title` and `current_guide.href` for the current guide.
This commit enables multiple table of contents rendering for the Solidus
Guides.
Each table of contents can have as many "submenus" as you want. Here is
the YAML structure for a short submenu:
```
-
- title: "Section name"
dropdown:
- title: "Article name"
href: "/relative/link/to/article.html"
- title: "Another article name"
href: "/relative/link/to/another-article.html"
```
Things you should note about the structure:
- Each submenu is represented by a root `-` node that is unnamed.
- Each submenu can have multiple sections and multiple articles.
- Articles need to be included in a `dropdown:` at this point in time.
\## Global and Meta tables of contents
The `/data/nav/global.yml` and `/data/nav/meta.yml` do not follow this
data structure. These are special menus that will be used throughout
the site and should not be rendered only when the user is on specific
pages.
Each table of contents provides a link to the `current_guide.href`. In the case of the "Solidus Developers Guide" table of contents, that links to `/developers/index.html`. These links should be styled to look like the <h1> of the table of contents block. This is similar to how the title of a blog is usually the color of a heading or the site's logo, not the color of a link.
It allows to access page as users/page.html instead of users/page like we do into other sections.
This was raising an error in the middleman console
by renaming product-stock.md into product-stock.html.md
kennyadsl
force-pushed
the
kennyadsl/guides/multiple_tocs
branch
from
1f225f8
to
038aea1
Compare
This is needed to switch between Developers and Users guides. This commit also changes the header structure a bit to accomodate this new navigation.
It also please Rubocop by removing Ruby 2.3 syntax which makes Hound to complain.
kennyadsl
force-pushed
the
kennyadsl/guides/multiple_tocs
branch
from
038aea1
to
97b676f
Compare
benjaminwil
approved these changes
Feb 12, 2019
spaghetticode
approved these changes
Feb 14, 2019
There was a problem hiding this comment.
@benjaminwil @kennyadsl thank you both for this great addition ❤️
jacobherrington
approved these changes
Feb 14, 2019
tvdeyen
approved these changes
Feb 14, 2019
There was a problem hiding this comment.
Very nice!
I just noticed that we do not have a cursor: pointer on any link. I think this is bad UI. Nothing we need to tackle in this PR, but the links in the sidebar and in the content have color or font-weight changes that indicate a hover state, the .nav-link in the .guides-nav do not have any visual indicator on :hover. Maybe fix this at least?
|
Thanks for the review @tvdeyen, but I can't find links without |
|
@kennyadsl Chrome I assume? In Firefox the links have no pointer. I guess they removed the default pointer for links and leave it to the site designers to use pointer cursors. Anyway, not important for this PR something we should think about later. |
|
@tvdeyen yes, Chrome. I agree we can tackle this into a separate PR. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR is a reopening of the great @benjaminwil work on #2770:
This PR adds some commit to actually start displaying Users Guides in our guides website, making its link accessible via a new navigation in the header.
Mobile
Desktop
NOTE
I know it does not look perfect on a UI/UX point of view. We'll work on that later if others think this is an acceptable solution for now.
Checklist: