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

fix(layouts): use the permalink instead of url #710

Merged
merged 1 commit into from
Mar 25, 2022

Conversation

james-d-elliott
Copy link
Contributor

Summary

Removes usage of deprecated .URL parameter, and brings the menus usage of .Permakink for href's to be consistent.

Basic example

I don't think a screenshot is relevant. But I modeled these changes after the usage of .Permalink here: https://github.com/h-enk/doks/blob/master/layouts/partials/sidebar/auto-collapsible-menu.html#L41

Motivation

Removes a deprecated usage of a parameter, suppresses an error, and makes the menu's slightly more consistent.

Checks

  • Read Create a Pull Request
  • Supports all screen sizes (if relevant)
  • Supports both light and dark mode (if relevant)
  • Passes npm run test

Copy link
Member

@h-enk h-enk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

Inspected + verified

@h-enk h-enk merged commit 84efb0e into gethyas:master Mar 25, 2022
@h-enk
Copy link
Member

h-enk commented Mar 25, 2022

I was a bit too quick. Tested now.

Your changes in this file (using .Page) were OK:

  • layouts/partials/sidebar/auto-default-menu.html

The changes in these ones (using .Menu) were NOK and produced errors:

  • layouts/partials/sidebar/manual-collapsible-menu.html
  • layouts/partials/sidebar/manual-default-menu.html

Note that, only .Page.URL is deprecated; .Menu.URL (and .Paginator.URL ) is not.

Fixed: 7e39fb6

@james-d-elliott
Copy link
Contributor Author

james-d-elliott commented Mar 25, 2022

I must have missed that! Thanks for pointing it out. It was just a bad assumption based on it generating the errors in auto-default-menu.

@james-d-elliott james-d-elliott deleted the fix-menu-link-perma branch March 30, 2022 09:54
adhadse pushed a commit to adhadse/doks that referenced this pull request Sep 17, 2022
fix(layouts): use the permalink instead of url
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.

None yet

2 participants