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

improve navigation highlighting: #211

Merged
merged 4 commits into from May 10, 2019

Conversation

Projects
None yet
3 participants
@salim-b
Copy link
Contributor

commented Apr 23, 2019

  • highlight blog section when on taxonomy sites (/tags/* and /categories/*)
  • highlight home, too (same as PR #208)

The blog section is identified by its identifier key, which has the advantage that users can change the name and url of the blog menu item.

There is one basic assumption behind the code: There is only one blog section in the whole site. I think this assumption is normally satisfied. If someone really wanted to create a site with multiple different blog sections, he/she would have to make other additional changes to this theme – which requires advanced knowledge of Hugo's internals anyway. For someone capable doing this, it should be rather trivial to also adjust the navigation partial to work as desired.

improve navigation highlighting:
- highlight blog section when on taxonomy sites (`/tags/` and `/categories/`)

- highlight home, too
@GeorgeWL
Copy link
Collaborator

left a comment

If at all possible, it would be good to have it set a default identifier for any menu items, possibly just a simple thing of it using the url with the slashes removed.

if not identifier theUrlStripped

@ryanfox1985

This comment has been minimized.

Copy link
Member

commented Apr 25, 2019

I don't like to force to the people to set 'blog' to a their post/blog/docs pages.

  • is it possible to change this condition? eq .Identifier "blog"
  • I dislike have another parameter idea.

for instance: https://linode.com/docs

@GeorgeWL GeorgeWL self-requested a review Apr 26, 2019

@GeorgeWL GeorgeWL self-requested a review Apr 26, 2019

@GeorgeWL
Copy link
Collaborator

left a comment

Don't make identifier a required field.

Make it instead a optional override of the identifier created from parsing url

@salim-b

This comment has been minimized.

Copy link
Contributor Author

commented Apr 26, 2019

If at all possible, it would be good to have it set a default identifier for any menu items, possibly just a simple thing of it using the url with the slashes removed.

if not identifier theUrlStripped

I don't understand what you mean... I've set the identifier key for all 4 menu items.

I don't like to force to the people to set 'blog' to a their post/blog/docs pages.

* is it possible to change this condition? `eq .Identifier "blog"`

* I dislike have another parameter idea.

for instance: https://linode.com/docs

I don't understand what you mean, either. We only "force" users to leave the identifier key untouched. They can still change the url and name as I've mentioned in the PR description.

@GeorgeWL

This comment has been minimized.

Copy link
Collaborator

commented Apr 29, 2019

I don't understand what you mean, either. We only "force" users to leave the identifier key untouched. They can still change the url and name as I've mentioned in the PR description.

It's still a required field, cause if for example the user creates new menu item, they must have a identifier param.

It's adding unnecessary steps, and would be better if it didn't need to be set but could be overridden.

@salim-b

This comment has been minimized.

Copy link
Contributor Author

commented Apr 29, 2019

It's still a required field, cause if for example the user creates new menu item, they must have a identifier param.
It's adding unnecessary steps, and would be better if it didn't need to be set but could be overridden.

Ok, now I get what you mean! You're right. I've added a fallback to trim .URL "/" if .Identifier is not set for a menu item. This makes setting the identifier key optional for new menu items.

@GeorgeWL

This comment has been minimized.

Copy link
Collaborator

commented May 3, 2019

Sounds good. I'd put an example into the exampleSite then I think it should be all good to pull right @ryanfox1985?

@salim-b

This comment has been minimized.

Copy link
Contributor Author

commented May 3, 2019

I'd put an example into the exampleSite

I'm afraid I don't understand what kind of example you mean. In the README I've added the following sentence:

Important: Do not change the identifier key of existing menu entries!

What else is there to mention?

@GeorgeWL

This comment has been minimized.

Copy link
Collaborator

commented May 6, 2019

There's a folder called examplesite in the GitHub, that's shows a working example of a site.

@salim-b

This comment has been minimized.

Copy link
Contributor Author

commented May 6, 2019

There's a folder called examplesite in the GitHub, that's shows a working example of a site.

Yeah, I know. But what do you want to add there?

@GeorgeWL GeorgeWL requested a review from ryanfox1985 May 10, 2019

@ryanfox1985
Copy link
Member

left a comment

thanks for contribute!

@ryanfox1985 ryanfox1985 merged commit 1291f43 into devcows:master May 10, 2019

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@GeorgeWL

This comment has been minimized.

Copy link
Collaborator

commented May 13, 2019

There's a folder called examplesite in the GitHub, that's shows a working example of a site.

Yeah, I know. But what do you want to add there?

I missed that you'd already done it 🤦‍♂

@salim-b salim-b deleted the salim-b:improve-menu branch May 14, 2019

salim-b added a commit to salim-b/hugo-universal-theme that referenced this pull request May 17, 2019

extend navigation highlighting to taxonomy list pages
This adds the same kind of navigation highlighting as the PRs devcows#191 and devcows#211
when on [taxonomy list pages](https://gohugo.io/content-management/taxonomies/#default-destinations)
(e.g. `/categories/programming/` or `/tags/hugo/`), i.e. the selected tag or
category will be highlighted in the color of the chosen style (default, blue,
green, etc.)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.