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

Headers: Use native sticky local navigation bar & site breadcrumbs #71

Merged
merged 2 commits into from
Nov 8, 2023

Conversation

ryelle
Copy link
Collaborator

@ryelle ryelle commented Oct 4, 2023

This will be necessary if WordPress/wporg-mu-plugins#466 merges, because the sticky behavior will change. This should make it easier to implement sticky bars on child themes, but does require updating the current sites that are using the older is-sticky class.

Other changes I made:

  • Removing the padding on local-navigation-bar, this is set correctly by default now
  • Move the border from local-navigation-bar to the global header, this looks better when the page scrolls
  • Since the local-navigation-bar was moved out of the group, I moved the style attribute to local-navigation-bar directly

Screenshots

This fixes an issue with the homepage scroll, and makes the breadcrumbs sticky.

Screen Before After
Home before-home-top after-home-top
Home scrolled before-home-scroll after-home-scroll
Article before-article-top after-article-top
Article scrolled before-article-scroll after-article-scroll

How to test the changes in this Pull Request:

  1. Apply the wporg-mu-plugins PR
  2. View any pages — the global header is no longer sticky, local nav is, and breadcrumbs (when they appear) are.

@adamwoodnz
Copy link
Contributor

adamwoodnz commented Nov 5, 2023

2. View any pages — the global header is no longer sticky, local nav is, and breadcrumbs (when they appear) are.

@fcoveram I believe the breadcrumbs should not be sticky, correct?

Sorry to keep repeating this question! Just not sure if we're going with that for the whole network. I think this is the outlier now, all others do not have sticky breadcrumbs :)

@adamwoodnz adamwoodnz force-pushed the update/sticky-header-breadcrumbs branch from 649591e to faff5ff Compare November 5, 2023 22:00
Copy link
Contributor

@adamwoodnz adamwoodnz left a comment

Choose a reason for hiding this comment

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

LGTM, just the one existing question on the sticky breadcrumbs.

@fcoveram
Copy link

fcoveram commented Nov 6, 2023

No worries @adamwoodnz for asking me again 😃 And yes, no sticky breadcrumb.

@adamwoodnz adamwoodnz merged commit 8add014 into trunk Nov 8, 2023
1 check passed
@adamwoodnz adamwoodnz deleted the update/sticky-header-breadcrumbs branch November 8, 2023 02:19
adamwoodnz added a commit that referenced this pull request Nov 8, 2023
adamwoodnz added a commit that referenced this pull request Nov 8, 2023
adamwoodnz added a commit that referenced this pull request Nov 8, 2023
adamwoodnz added a commit that referenced this pull request Nov 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Component] Theme Templates, patterns, CSS
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants