Skip to content

Improve search visibility and styling#522

Open
rohIta-k wants to merge 10 commits into
krkn-chaos:mainfrom
rohIta-k:krkn/search-visibility-improvements
Open

Improve search visibility and styling#522
rohIta-k wants to merge 10 commits into
krkn-chaos:mainfrom
rohIta-k:krkn/search-visibility-improvements

Conversation

@rohIta-k

@rohIta-k rohIta-k commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

Changes

  • Increased visibility of the sidebar search input and the results modal.
  • Added consistent border and border-radius styling to search results containers.
  • Prevented duplicate search result popovers from being rendered when switching between sidebar search and navbar search.

Related Issue

Attached Before and After screenshots in the issue.

Signed-off-by: Rohita <rohitak.srimayee@gmail.com>
@netlify

netlify Bot commented Jun 5, 2026

Copy link
Copy Markdown

Deploy Preview for krkn-chaos ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit a8a08c7
🔍 Latest deploy log https://app.netlify.com/projects/krkn-chaos/deploys/6a54f616ab42c40008e33317
😎 Deploy Preview https://deploy-preview-522--krkn-chaos.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.

@qodo-code-review

Copy link
Copy Markdown

Review Summary by Qodo

✨ Enhancement

Grey Divider

Walkthroughs

Description
• Prevent duplicate search result popovers when switching between search inputs
• Increase search input visibility with darker borders and consistent styling
• Enhance search results modal with improved border styling and visibility
• Add dark theme styling for form controls and search inputs
Diagram
flowchart LR
  A["Search Input"] -->|"Remove duplicate popovers"| B["Improved Visibility"]
  C["Form Controls"] -->|"Add dark theme styling"| D["Enhanced Styling"]
  E["Search Results Modal"] -->|"Increase border thickness"| F["Better Visual Hierarchy"]
  B --> G["Better UX"]
  D --> G
  F --> G

Loading

Grey Divider

File Changes

1. layouts/_partials/navbar.html 🐞 Bug fix +12/-3

Prevent duplicate search popovers on open

• Remove existing Docsy search popovers before opening search overlay to prevent duplicates
• Improve code formatting with consistent spacing and function declarations
• Ensure search input receives focus after CSS transition completes

layouts/_partials/navbar.html


2. assets/scss/_custom_docs.scss ✨ Enhancement +22/-4

Add dark theme search input styling

• Add dark theme styling for form controls and search inputs with primary border color
• Increase search input border thickness to 1.5px for better visibility
• Update form placeholder text color to use muted text variable
• Improve code formatting and comment placement for readability

assets/scss/_custom_docs.scss


3. assets/scss/_custom_navbar.scss ✨ Enhancement +7/-3

Enhance search results container borders

• Increase search results dropdown border from 1px to 2px for improved visibility
• Enhance offline search results popover with thicker borders and rounded bottom corners
• Add border styling to search results container with proper border-top removal
• Improve visual consistency across search result containers

assets/scss/_custom_navbar.scss


Grey Divider

Qodo Logo

@qodo-code-review

qodo-code-review Bot commented Jun 5, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (2) 📘 Rule violations (0) 📎 Requirement gaps (0)

Context used
✅ Compliance rules (platform): 24 rules

Grey Divider


Remediation recommended

1. Global form styles overridden 🐞 Bug ≡ Correctness
Description
assets/scss/_custom_docs.scss adds a [data-theme="dark"] rule that applies !important
background/border styles to all .form-control/.form-select, not just doc search inputs,
overriding the project’s baseline form and focus styling across the whole site when dark theme is
active.
Code

assets/scss/_custom_docs.scss[R5-17]

+[data-theme="dark"] {
+  .form-control,
+  .form-select,
+  .td-search-input,
+  .td-search input[type="search"] {
+    background-color: var(--krkn-surface) !important;
+    border: 1.5px solid var(--krkn-primary) !important;
+    color: var(--krkn-text) !important;
+
+    &::placeholder {
+      color: var(--krkn-text-muted) !important;
+    }
+  }
Evidence
The new rule applies to .form-control/.form-select under [data-theme="dark"] with
!important, which overrides the baseline .form-control/.form-select and :focus styling defined
in _styles_project.scss. Since custom_docs is imported into the global SCSS bundle and the
data-theme attribute is set on document.documentElement, the rule impacts all pages in dark
mode, not just docs.

assets/scss/_custom_docs.scss[5-17]
assets/scss/_styles_project.scss[247-260]
assets/scss/_styles_project.scss[412-420]
layouts/_partials/hooks/head-end.html[1-6]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
A new `[data-theme="dark"]` rule in `assets/scss/_custom_docs.scss` targets generic Bootstrap selectors (`.form-control`, `.form-select`) and uses `!important` for `background-color` and `border`. Because the site sets `data-theme` on `documentElement` globally and `custom_docs` is imported into the global stylesheet bundle, this override can unintentionally restyle all form controls across the website and also suppress existing focus-state styling.

## Issue Context
This PR intends to improve search input visibility (Docsy search) in docs/sidebar and the navbar modal. The current selector list is broader than necessary.

## Fix
Limit the selector scope to Docsy search inputs / docs containers (e.g., `.td-sidebar`, `.td-main`, `.td-content`, `.td-search`), and avoid using `border: ... !important` on generic `.form-control` unless that is explicitly desired site-wide.

## Fix Focus Areas
- assets/scss/_custom_docs.scss[5-17]
- assets/scss/_styles_project.scss[247-260]
- assets/scss/_styles_project.scss[412-420]
- layouts/_partials/hooks/head-end.html[1-6]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. Arrow function in inline JS 🐞 Bug ☼ Reliability
Description
openSearch() uses an ES6 arrow function (el => el.remove()) in an otherwise ES5-style inline
script, risking runtime failure on older browsers where arrow functions (and sometimes
NodeList.forEach) aren’t supported, which would break opening the search overlay.
Code

layouts/_partials/navbar.html[R203-206]

+    // Remove any existing Docsy search popovers
+    document
+      .querySelectorAll('.td-offline-search-results')
+      .forEach(el => el.remove());
Evidence
The only arrow function in the partial is introduced in openSearch(), while the rest of the same
script uses ES5-style functions; the project’s root package.json also doesn’t show a JS
transpilation toolchain that would rewrite inline scripts for compatibility.

layouts/_partials/navbar.html[176-180]
layouts/_partials/navbar.html[200-217]
package.json[47-52]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The inline navbar script is written in ES5 style (`var`, `function`), but the PR introduces an ES6 arrow function in `openSearch()`. Inline scripts are shipped directly to the browser (not evidently transpiled), so this can break execution on environments without ES6 support.

## Issue Context
The goal is to remove existing `.td-offline-search-results` nodes before opening the navbar search overlay.

## Fix
Replace the arrow callback with an ES5 `function(el) { ... }` and (optionally) replace `NodeList.forEach` with a `for` loop for maximum compatibility.

## Fix Focus Areas
- layouts/_partials/navbar.html[199-217]
- package.json[47-52]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Qodo Logo

@github-actions github-actions Bot added the needs-dco Commits are missing a Signed-off-by line label Jun 5, 2026
…ity-improvements

Signed-off-by: Rohita <rohitak.srimayee@gmail.com>
@rohIta-k rohIta-k force-pushed the krkn/search-visibility-improvements branch from d610638 to d2e356c Compare June 5, 2026 16:02
@github-actions github-actions Bot removed the needs-dco Commits are missing a Signed-off-by line label Jun 5, 2026
@paigerube14

Copy link
Copy Markdown
Contributor
Screenshot 2026-06-05 at 12 03 05 PM

In your description you stated "Prevented duplicate search result popovers from being rendered when switching between sidebar search and navbar search." I was still able to open the search bar and have a side bar search. I opened the top level search fist and then the left one. Not sure if you fixed for one certain order but could be nice to have the other way too. Wondering if top level search is open if we should deactivate left search and vice versa.

Thanks so much for this PR, appreciate your contributions

@rohIta-k

rohIta-k commented Jun 5, 2026

Copy link
Copy Markdown
Contributor Author

Hi @paigerube14, I've submitted this PR and would appreciate a review when convenient. Please let me know if any changes are needed.

Signed-off-by: Rohita <rohitak.srimayee@gmail.com>
@rohIta-k

rohIta-k commented Jun 6, 2026

Copy link
Copy Markdown
Contributor Author

Sorry for not catching this earlier. After looking at it further, I realized I was only testing at normal zoom levels.

At normal zoom, the issue was masked because clicking the sidebar search causes the navbar search overlay to close via the backdrop click handler, so duplicate search UIs are not visible. After increasing the browser zoom, I was able to reproduce the same overlapping search behavior shown in your screenshot.

To address this, I updated the search handling so that the behavior no longer depends on the backdrop interaction:

  • In openSearch(), any existing search popovers are removed before opening the navbar search. This handles the sidebar → navbar flow.
  • In closeSearch(), the navbar search overlay is properly cleared when switching away from the navbar search. This handles the navbar → sidebar flow.

I've pushed the latest changes addressing the issue. Whenever you have a chance, could you please take another look and let me know if everything looks good now?

@github-actions github-actions Bot added the needs-dco Commits are missing a Signed-off-by line label Jun 9, 2026
Lovlace777 and others added 2 commits June 9, 2026 14:45
Signed-off-by: Lovelace <61972457+Lovlace777@users.noreply.github.com>
Signed-off-by: Rohita <rohitak.srimayee@gmail.com>
Signed-off-by: Paige Patton <prubenda@redhat.com>
Signed-off-by: Rohita <rohitak.srimayee@gmail.com>
@rohIta-k rohIta-k force-pushed the krkn/search-visibility-improvements branch from 3491367 to 04e83c1 Compare June 9, 2026 09:16
@github-actions github-actions Bot removed the needs-dco Commits are missing a Signed-off-by line label Jun 9, 2026
Signed-off-by: Rohita <rohitak.srimayee@gmail.com>
@rohIta-k

rohIta-k commented Jun 9, 2026

Copy link
Copy Markdown
Contributor Author

@paige I've updated my branch with the latest changes from main. Could you please review the PR again when you have a chance? Thanks!

@rohIta-k

Copy link
Copy Markdown
Contributor Author

Updated the PR with the latest changes. Ready for review!

@github-actions github-actions Bot added the needs-dco Commits are missing a Signed-off-by line label Jun 10, 2026
* Fix Mermaid theme selection based on site theme

Signed-off-by: Rohita <rohitak.srimayee@gmail.com>

* fix(mermaid): rerender diagrams on theme change

Signed-off-by: Rohita <rohitak.srimayee@gmail.com>

* Fix Mermaid theme toggle rendering

Signed-off-by: Rohita <rohitak.srimayee@gmail.com>

---------

Signed-off-by: Rohita <rohitak.srimayee@gmail.com>
@rohIta-k rohIta-k force-pushed the krkn/search-visibility-improvements branch from f17aa9e to ae2a971 Compare June 10, 2026 17:19
@github-actions github-actions Bot removed the needs-dco Commits are missing a Signed-off-by line label Jun 10, 2026
…ity-improvements

Signed-off-by: Rohita <rohitak.srimayee@gmail.com>
@rohIta-k

Copy link
Copy Markdown
Contributor Author

I've merged the latest upstream changes and updated the branch. Can you review it again? Thanks!

@github-actions

Copy link
Copy Markdown

Hi @rohIta-k! 👋 One or more commits in this PR are missing a Signed-off-by line, which is required by our DCO policy.

Unsigned commits:

  • a8a08c7: Merge branch 'main' into krkn/search-visibility-improvements

To fix, rebase and sign off all commits:

git rebase HEAD~1 --signoff
git push --force-with-lease

Or, to sign off every commit in this PR at once:

git rebase origin/main --signoff
git push --force-with-lease

Tip: Add -s to future git commit commands (e.g. git commit -s -m "...") to sign off automatically.

Note: Please make sure your PR contains only 1 commit before signing off. If you have multiple commits, squash them first:

git checkout <my-working-branch>
git rebase -i HEAD~<num_of_commits_to_merge>
   -OR-
git rebase -i <commit_id_of_first_change_commit>

In the interactive rebase screen, set the first commit to pick and all others to squash. Then push your rebased commits:

git push origin <my-working-branch> --force-with-lease

See the Squash Commits guide for more details.

The needs-dco label and this comment will be removed automatically once all commits are signed off.

@github-actions github-actions Bot added the needs-dco Commits are missing a Signed-off-by line label Jul 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-dco Commits are missing a Signed-off-by line

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Improve Search Input Visibility and Visual Consistency

3 participants