Skip to content

Comments

docs: prevent header tabs from overlapping on search expand#6556

Draft
jijo-OO7 wants to merge 13 commits intoknative:mainfrom
jijo-OO7:docs/fix-header-tabs-overlap
Draft

docs: prevent header tabs from overlapping on search expand#6556
jijo-OO7 wants to merge 13 commits intoknative:mainfrom
jijo-OO7:docs/fix-header-tabs-overlap

Conversation

@jijo-OO7
Copy link
Contributor

@jijo-OO7 jijo-OO7 commented Jan 14, 2026

Header tabs overlap when search expands [Fixes #6332]

problem

Proposed Changes

  • Fixed navigation layout shift when search is activated
  • Restructured header to use separate sticky navigation container
  • Maintained responsive design across all breakpoints

Reference: https://squidfunk.github.io/mkdocs-material/tutorials/

@netlify
Copy link

netlify bot commented Jan 14, 2026

Deploy Preview for knative ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit a9e322a
🔍 Latest deploy log https://app.netlify.com/projects/knative/deploys/699b52f8ba00a90008aa32d0
😎 Deploy Preview https://deploy-preview-6556--knative.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@knative-prow
Copy link

knative-prow bot commented Jan 14, 2026

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: jijo-OO7
Once this PR has been reviewed and has the lgtm label, please assign dprotaso for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@knative-prow knative-prow bot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Jan 14, 2026
@lakxy
Copy link
Contributor

lakxy commented Jan 14, 2026

Hi @jijo-OO7 I don't think you have actually followed the issue discussion, read it then open the PR.

@lakxy
Copy link
Contributor

lakxy commented Jan 14, 2026

This is totally mis-aligned from what @dprotaso described there.

@dprotaso
Copy link
Member

Yeah I don't see any difference on Chrome Desktop.

@dprotaso
Copy link
Member

Note the idea is for the home, docs etc in the top row to not move when the search bar has focus

@jijo-OO7
Copy link
Contributor Author

Thanks for the pointer. I’ve re-read the original issue discussion and I see where the intent differs from the current implementation.
Based on @dprotaso’s clarification, I’ll update the PR so the top navigation remains static when the search bar gains focus

@knative-prow knative-prow bot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Jan 15, 2026
@jijo-OO7
Copy link
Contributor Author

Hi @dprotaso Updated the implementation based on your feedback. The top navigation now remains static when search gains focus by moving to a separate sticky container, following the Material for MkDocs pattern.

Would appreciate your review to ensure this matches the intended behavior. Let me know if any further adjustments are needed.

@lakxy
Copy link
Contributor

lakxy commented Jan 15, 2026

@jijo-OO7 I am not sure if this is what was described? the before version is not complete, initially there were elements in nav beside the search bar, which have been removed in after version,
You explicitly hid that part in PR description? why?

@lakxy
Copy link
Contributor

lakxy commented Jan 15, 2026

Final review to @dprotaso . Does not seem optimal to me.

@lakxy
Copy link
Contributor

lakxy commented Jan 15, 2026

Thanks for the pointer. I’ve re-read the original issue discussion and I see where the intent differs from the current implementation. Based on @dprotaso’s clarification, I’ll update the PR so the top navigation remains static when the search bar gains focus

@jijo-OO7 Again, you did not follow the issue discussion, as Far as I know the nav elements were described to be overlayed by search bar expansion and not removed.

@dprotaso
Copy link
Member

Yeah this doesn't look correct

Screenshot 2026-01-15 at 12 33 10 PM

@jijo-OO7
Copy link
Contributor Author

@dprotaso Could you try reloading the preview? The navigation elements remain visible on my local build and the search expands as expected per the Material for MkDocs pattern. A cache refresh might resolve any discrepancies.
Screenshot (19)

@dprotaso
Copy link
Member

I would expect Home etc. to be at the same horizontal line as the search bar

@jijo-OO7
Copy link
Contributor Author

Got it, thank you for clarifying. I understand now, the navigation items should remain aligned on the same horizontal line as the search bar. I'll revisit the implementation to ensure proper alignment and will push an updated commit shortly with Home, Docs, About, Blog, and Community properly aligned alongside the search bar.

@jijo-OO7
Copy link
Contributor Author

Do we expect the top nav to be static as well upon search expansion ?

@dprotaso
Copy link
Member

Do we expect the top nav to be static as well upon search expansion ?

Yeah

@jijo-OO7 jijo-OO7 force-pushed the docs/fix-header-tabs-overlap branch from 6b64dc1 to cfe46da Compare January 31, 2026 07:47
@knative-prow knative-prow bot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jan 31, 2026
@jijo-OO7
Copy link
Contributor Author

Hi @dprotaso, I’ve updated the PR with the latest changes. Could you please take a look when you have a moment and let me know if this aligns with the intended vision, or if any further tweaks are needed?

@jijo-OO7
Copy link
Contributor Author

Fixed a broken redirect for the contributing docs that was causing the strict docs build to fail after rebasing onto the latest upstream/main

@jijo-OO7 jijo-OO7 force-pushed the docs/fix-header-tabs-overlap branch from 3193b37 to 8826d64 Compare February 5, 2026 10:10
@jijo-OO7
Copy link
Contributor Author

jijo-OO7 commented Feb 5, 2026

@dprotaso I ran ./hack/build.sh --strict locally and the build fails with:
WARNING - Redirect target 'community/contributing/README.md' does not exist!

I verified the current docs structure and we have docs/community/contributing.md (file), not docs/community/contributing/README.md (directory + file). I tried updating the redirect target to community/contributing.md, but the strict check still fails.

This makes me think the CI check might be validating redirects against a different path layout (or across release branches where the structure differs). Could you confirm the expected redirect target here, or whether the strict check needs an update to reflect the current docs structure?

@jijo-OO7
Copy link
Contributor Author

jijo-OO7 commented Feb 10, 2026

@dprotaso i have updated the pr with expected changes .
can you check on this once on your end .
Thank you for your time and patience.

@dprotaso
Copy link
Member

I see the menu doesn't move but everything else is broken.

@jijo-OO7 At this point I don't want to keep reviewing this PR while things remain in a broken state. This has happened on about 10 times now.

I'm moving this back to draft and I want you to verify things are working as expected prior to marking this as done. To make open source managable I can't be 'testing' your changes when clearly you have not.

@dprotaso dprotaso marked this pull request as draft February 10, 2026 23:28
@knative-prow knative-prow bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Feb 10, 2026
@dprotaso
Copy link
Member

Everytime you push a change the preview is updated here: #6556 (comment)

You should be using that to verify your work.

@jijo-OO7
Copy link
Contributor Author

jijo-OO7 commented Feb 11, 2026

I see the menu doesn't move but everything else is broken.

@jijo-OO7 At this point I don't want to keep reviewing this PR while things remain in a broken state. This has happened on about 10 times now.

I'm moving this back to draft and I want you to verify things are working as expected prior to marking this as done. To make open source managable I can't be 'testing' your changes when clearly you have not.

I understand the concern, and apologies for the repeated broken states I’ll make sure this is properly validated against the preview going forward before marking it ready again.

@jijo-OO7
Copy link
Contributor Author

I’ve made some progress on reducing the overlap behavior, but resolving this cleanly across viewports is turning into a larger layout refactor than initially anticipated.

To avoid blocking other work, I’m going to pause on this for now. I’m happy to revisit and iterate further if this is still considered a priority.

Thanks!

@jijo-OO7 jijo-OO7 force-pushed the docs/fix-header-tabs-overlap branch from 99d4d03 to 04450bf Compare February 22, 2026 13:35
@jijo-OO7
Copy link
Contributor Author

UPDATE

I revisited this and scoped the header/search layout fixes to desktop breakpoints only, so mobile behavior remains unchanged.
I’ve verified the overlap is resolved on desktop and that mobile viewports behave as before (including on deploy preview).

@jijo-OO7 jijo-OO7 marked this pull request as ready for review February 22, 2026 13:55
@knative-prow knative-prow bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Feb 22, 2026
@knative-prow knative-prow bot requested a review from skonto February 22, 2026 13:56
@dprotaso dprotaso marked this pull request as draft February 22, 2026 15:21
@knative-prow knative-prow bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Feb 22, 2026
@dprotaso
Copy link
Member

The search functionality is still broken - please test your changes before marking the PR as ready

@jijo-OO7
Copy link
Contributor Author

jijo-OO7 commented Feb 22, 2026

Hi @dprotaso — thanks for the continued feedback and for your patience here.

I’ve been rechecking the latest Netlify preview and confirmed the search interaction is still not behaving as expected with my recent changes. I’m keeping this in draft until that’s resolved.

While debugging, I also noticed that even after reverting my CSS updates, the search remains in an “Initializing search” state in:

Docker preview (./hack/docker/run.sh)

Netlify preview

Since this differs from production, I might be missing something in the preview/dev setup. If there’s a known setup or reference branch you’d recommend for validating search behavior locally, I’d appreciate the pointer.

Thanks again for the guidance.

@dprotaso
Copy link
Member

unsure @evankanderson do you know why the search might be broken in the preview?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

When search bar is expanded it causes top section titles to be scrunched

3 participants