Skip to content

fix(docs): prevent search overlay overlapping branding on mobile Closes #1660 #1661

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

Mohit5Upadhyay
Copy link

Description

This PR fixes issue #1660 a mobile view bug where the search overlay was overlapping the “npm docs” logo, causing a cluttered visual appearance and also the search button will remain visible even when we click the search Button.

Actual

issue-search-box.mp4

Updated

issue-resolved-search.mp4

References

Closes #1660

@Mohit5Upadhyay
Copy link
Author

Hey team 👋

This PR addresses #1660 by updating the CSS & handleClick to prevent the search overlay from overlapping the npm docs logo on mobile devices. I’ve tested it in both mobile and desktop views to ensure everything looks good.

Let me know if you’d like any changes or if there’s a preferred approach for handling the header during search.

Thanks so much for taking the time to review this!

@owlstronaut owlstronaut self-requested a review July 8, 2025 17:34
@owlstronaut owlstronaut self-assigned this Jul 8, 2025
Copy link
Contributor

@owlstronaut owlstronaut left a comment

Choose a reason for hiding this comment

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

@Mohit5Upadhyay This is great, thank you! I have just a couple ideas that would make this a little less heavy-handed on the z-index and more extensible for later

For constants we could add this:

export const Z_INDEX = {
  HEADER: 10,
  SEARCH_OVERLAY: 25
}

Then we can do

import {HEADER_HEIGHT, Z_INDEX, HEADER_BAR} from '../constants'

...

<FocusOn returnFocus={true} onEscapeKey={() => resetAndClose(true)}>
  <Box
    sx={{
...
      zIndex: Z_INDEX.SEARCH_OVERLAY,
    }}
  >
            
...
<Box sx={{display: 'flex', flexDirection: 'column', height: resultsOpen ? '100%' : 'auto'}}>
  <Box
    sx={{
...
      zIndex: Z_INDEX.SEARCH_OVERLAY + 1, 
    }}
  > 

And a quick edit of the header.js file

import {HEADER_HEIGHT, HEADER_BAR, Z_INDEX} from '../constants'
...
<DarkTheme sx={{top: 0, position: 'sticky', zIndex: Z_INDEX.HEADER}}>

If you rebase on main, the linter should start passing for you now as well (though the publisher won't currently work for fork prs)

@Mohit5Upadhyay Mohit5Upadhyay force-pushed the fix-1660-search-bar-overlap branch from 94a06db to 099221c Compare July 9, 2025 07:56
@Mohit5Upadhyay Mohit5Upadhyay requested a review from a team as a code owner July 9, 2025 07:56
@Mohit5Upadhyay
Copy link
Author

Hi @owlstronaut 👋
I’ve pushed the changes after rebasing and implementing your feedback (using z-index constants).
Let me know if there’s anything else you’d like adjusted.

Thanks!

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.

Bug: Search NPM Docs is overlapping over the NPM Docs branding in mobile view
2 participants