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

Sticky Header option bug #2935

Closed
2 tasks done
Agos95 opened this issue Apr 20, 2023 · 4 comments · Fixed by #2950
Closed
2 tasks done

Sticky Header option bug #2935

Agos95 opened this issue Apr 20, 2023 · 4 comments · Fixed by #2950

Comments

@Agos95
Copy link
Contributor

Agos95 commented Apr 20, 2023

Prerequisites

  • I have searched for duplicate or closed feature requests
  • I am mindful of the project scope

Proposal

My proposal is to change this code

https://github.com/wowchemy/wowchemy-hugo-themes/blob/57e374fe08d8bb0eee34b160d479a77dc13b8503/modules/wowchemy/layouts/partials/site_js.html#L131

In fact, instead of looking at

header:
  on_scroll:

actually the code searches for

header.on_scroll: 

I suggest to:

  • use the keyword sticky instead of on_scroll, which in my opinion is more clear.
  • search for
    header:
      sticky: true/false

option to configure the header. Now, the value must be set to "disappear" to have a disappearing header, while everything else creates a sticky one. The use of true/false instead may be more clear.

  • possibility to set a global behaviour using the params.yaml configuration file, or a per page one by using the single page Front Matter.
  • backward compatibility with the old "buggy" way to set the sticky header with "header.on_scroll"

The logic to set the behaviour of the header should be:

  1. by default homepage and book layout have a sticky header and all other pages have a disappearing one
  2. use the global configuration in params.yaml to set the options to the whole site:
header:
  sticky: true/false
  1. use the specific page option, by looking the front matter:
header:
  sticky: true/false

# or this one for backword compatibility
#header.on_scroll: 

I already fix this behaviour in my fork. If you think this can be useful, I can proceed with a PR.

Motivation and context

As stated in this discussion on Discord, there is a bug in the syntax to set the header sticky. In particular, this is the buggy code:

https://github.com/wowchemy/wowchemy-hugo-themes/blob/57e374fe08d8bb0eee34b160d479a77dc13b8503/modules/wowchemy/layouts/partials/site_js.html#L131

As for now, to make the header sticky you need to set the parameter

header.on_scroll : false

but looking at the code it should be:

header:
  on_scroll : false

In addition, it actually checks if sticky.header is set and equal to "disappear", but I think this should be a boolean value.

@github-actions
Copy link

This issue is stale because it has not had any recent activity. The resources of the project maintainers are limited, and so we are asking for your help.

If this is a bug and you can still reproduce this error on the main branch, consider contributing a Pull Request with a fix.

If this is a feature request, and you feel that it is still relevant and valuable, consider contributing a Pull Request for review.

This issue will automatically close soon if no further activity occurs. Thank you for your contributions.

@github-actions github-actions bot added the stale label May 21, 2023
@rodrigoalcarazdelaosa
Copy link
Contributor

I suggest you submit a PR, @Agos95

@Agos95
Copy link
Contributor Author

Agos95 commented May 21, 2023

Ok, I submitted a PR!

@github-actions github-actions bot removed the stale label May 22, 2023
@github-actions
Copy link

This issue is stale because it has not had any recent activity. The resources of the project maintainers are limited, and so we are asking for your help.

If this is a bug and you can still reproduce this error on the main branch, consider contributing a Pull Request with a fix.

If this is a feature request, and you feel that it is still relevant and valuable, consider contributing a Pull Request for review.

This issue will automatically close soon if no further activity occurs. Thank you for your contributions.

@github-actions github-actions bot added the stale label Jun 22, 2023
gcushen pushed a commit that referenced this issue Jul 30, 2023
…2950)

It has been reported that recent Hugo versions broke user's configuration of Headroom JS via the `header.on_scroll` option.

With this PR, Headroom JS configuration is working again.

Fixes #2935
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.

2 participants