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.

Sign up for GitHub

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

Jump to bottom

Update header behaviour #334

Merged
merged 3 commits into from
Nov 8, 2023
Merged

Update header behaviour #334

merged 3 commits into from
Nov 8, 2023

Conversation

adamwoodnz
Copy link
Contributor

@adamwoodnz adamwoodnz commented Nov 6, 2023 •
edited
Loading

Fixes #298
Depends on WordPress/wporg-mu-plugins#466

Very similar to changes required for Documentation after the global header becomes non-sticky:

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

With these changes only the local nav is sticky (global header, breadcrumbs and search are not).

Screenshots

Page Top Scrolled
Home localhost_8888_(Desktop) (10) localhost_8888_(Desktop) (11)
With breadcrumbs localhost_8888_coding-standards_wordpress-coding-standards_css_(Desktop) localhost_8888_coding-standards_wordpress-coding-standards_css_(Desktop) (1)
Without breadcrumbs localhost_8888_cli_commands_(Desktop) (4) localhost_8888_cli_commands_(Desktop) (5)

Testing

  1. Ensure your mu-plugins is on Global Header: Try unsticking the global header, making local navigation sticky wporg-mu-plugins#466
  2. Navigate to the pages in the screenshots above at desktop size, and check the header behaviour.
  3. Resize for tablet and mobile; no headers should be sticky below 890px (tablet landscape will still have sticky local nav).

@adamwoodnz adamwoodnz added this to the MVP milestone Nov 6, 2023
@adamwoodnz adamwoodnz self-assigned this Nov 6, 2023
@adamwoodnz adamwoodnz force-pushed the fix/298-sticky-headers branch from 067754c to 2c03ac5 Compare November 6, 2023 00:54
@adamwoodnz adamwoodnz force-pushed the fix/298-sticky-headers branch from 2c03ac5 to e5f027c Compare November 6, 2023 00:56
@adamwoodnz adamwoodnz marked this pull request as ready for review November 6, 2023 01:20
@adamwoodnz adamwoodnz mentioned this pull request Nov 6, 2023
Merged
renintw
renintw approved these changes Nov 6, 2023
View reviewed changes
Copy link
Contributor

@renintw renintw left a comment

Choose a reason for hiding this comment

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

Behaves as described. LGTM 👍

@adamwoodnz adamwoodnz merged commit 0955198 into trunk Nov 8, 2023
1 check passed
@adamwoodnz adamwoodnz deleted the fix/298-sticky-headers branch November 8, 2023 02:19
adamwoodnz added a commit that referenced this pull request Nov 8, 2023
@adamwoodnz
Revert "Update header behaviour (#334)"
f6b8e6c 
This reverts commit 0955198.
@adamwoodnz adamwoodnz mentioned this pull request Nov 8, 2023
Merged
adamwoodnz added a commit that referenced this pull request Nov 8, 2023
@adamwoodnz
Revert "Update header behaviour" (#347)
f2a0a52 
Revert "Update header behaviour (#334)"

This reverts commit 0955198.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@renintw renintw renintw approved these changes

@StevenDufresne StevenDufresne Awaiting requested review from StevenDufresne

Assignees

@adamwoodnz adamwoodnz

Labels
Redesign
Projects
None yet
Milestone
MVP
Development

Successfully merging this pull request may close these issues.

Fix sticky local nav and breadcrumbs after global header update
2 participants
@adamwoodnz @renintw