Skip to content

Added search box hide button#122

Open
satyansh2004 wants to merge 2 commits intoprem-k-r:mainfrom
satyansh2004:added-search-box-hide-button
Open

Added search box hide button#122
satyansh2004 wants to merge 2 commits intoprem-k-r:mainfrom
satyansh2004:added-search-box-hide-button

Conversation

@satyansh2004
Copy link

@satyansh2004 satyansh2004 commented Jan 14, 2026

📌 Description

This PR adds a button to hide the search box, allowing users who don’t use the search feature to keep the interface clean and minimal.


🎨 Visual Changes (Screenshots / Videos)

image
Screenshot 2026-01-14 162539

🔗 Related Issue


✅ Checklist

  • I have read and followed the Contributing Guidelines.
  • My code follows the project's coding style and conventions.
  • I have tested my changes thoroughly to ensure expected behavior.
  • I have verified compatibility across Chrome and Firefox.
  • I have attached relevant visual evidence (screenshots/videos) if applicable.
  • I have updated the CHANGELOG.md

Summary

This PR implements a feature to allow users to hide the search box via a toggle switch, addressing issue #51. The implementation includes changes to index.html, scripts/search.js, and CHANGELOG.md.

Implementation Issues

Critical Bug in search.js (Line 449):
The initialization code contains a variable scope bug in the conditional logic:

const isSearchBarswitchEnabled = localStorage.getItem("showSearchBarswitch").toString() === "true";
// ...
if (isSearchBarswitchEnabled) {
    hideSearchbar();
} else if (!isShortCutSwitchEnabled) {  // BUG: References wrong variable
    showSearchbar();
}

The condition should check !isSearchBarswitchEnabled instead of !isShortCutSwitchEnabled. This causes the searchbar visibility to depend on an unrelated shortcut switch state rather than its own state.

Conflicting HTML Styling:
In index.html (line 673), the searchbar container includes an inline style visibility: visible; which makes the element always visible regardless of JavaScript state changes. This directly conflicts with the JavaScript logic that attempts to control visibility using visibility: hidden/visible. The inline style will override the JavaScript's visibility toggles.

Duplicate Code:
The initialization logic for the search bar switch (approximately lines 420-456 in search.js) contains duplicate blocks, resulting in redundant execution of the same code at the end of the file.

Missing UI Affordance:
While a checkbox element with ID searchBoxCheckBox is referenced in the code, the PR description mentions adding a "hide button," but no visible button element is evident in the markup. The control appears to be a settings checkbox rather than a dedicated button interface.

Additional Changes

  • CHANGELOG.md: Correctly documents the new feature on line 110 under the "Added" section.
  • localStorage Integration: The feature persists the user's preference via localStorage with the key showSearchBarswitch.
  • Helper Functions: Two utility functions (showSearchbar() and hideSearchbar()) control visibility by manipulating the element's visibility style property.

Recommendations for Review

  1. Resolve the variable scope bug on line 449 by using the correct variable reference
  2. Remove the conflicting inline style="visibility: visible;" from the searchbar div in index.html
  3. Remove or refactor the duplicate initialization code
  4. Verify the UI control (checkbox vs. button) matches the stated functionality and is appropriately labeled and discoverable

Copilot AI review requested due to automatic review settings January 14, 2026 10:57
@coderabbitai
Copy link

coderabbitai bot commented Jan 14, 2026

📝 Walkthrough

Walkthrough

This PR implements a user-requested feature to toggle search box visibility. The implementation adds a checkbox control that shows/hides the search bar and persists the user's preference in localStorage. Changes include documentation, HTML markup styling, and JavaScript toggle logic with state persistence.

Changes

Cohort / File(s) Summary
Documentation
CHANGELOG.md
Added changelog entry documenting the new search box hide feature.
HTML Structure
index.html
Added explicit inline visibility: visible styles to the searchbar container in two locations to ensure visibility state is always controllable.
Search Bar Toggle Logic
scripts/search.js
Implemented checkbox-based toggle functionality for search bar visibility with localStorage persistence. Includes showSearchbar() and hideSearchbar() functions, initSearchBarswitch() for state initialization, and change listener on checkbox. Note: Code block duplicated at end of file.

Sequence Diagram

sequenceDiagram
    participant User
    participant Browser as Browser/DOM
    participant JS as JavaScript
    participant Storage as localStorage

    User->>Browser: Click checkbox
    Browser->>JS: Trigger change event
    JS->>JS: Check checkbox state
    alt Checkbox enabled
        JS->>Browser: Show search bar (visibility: visible)
    else Checkbox disabled
        JS->>Browser: Hide search bar (visibility: hidden)
    end
    JS->>Storage: Save preference (showSearchBarswitch)
    
    Note over User,Storage: On page reload:
    JS->>Storage: Read showSearchBarswitch preference
    Storage-->>JS: Return saved state
    JS->>Browser: Apply saved visibility state
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title 'Added search box hide button' clearly summarizes the main feature being introduced in this pull request.
Description check ✅ Passed The description follows the template with all required sections: clear description, visual evidence (screenshots), related issues reference, and completed checklist.
Linked Issues check ✅ Passed The PR successfully implements the feature requested in issue #51: a search box hide button allowing users to toggle visibility of the search box.
Out of Scope Changes check ✅ Passed All changes are directly related to implementing the search box hide button feature. Changes include the feature implementation, CHANGELOG update, and necessary UI/styling modifications.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR implements a toggle feature that allows users to hide the search box from the interface, providing a cleaner UI for users who don't utilize the search functionality. The feature persists the user's preference in localStorage.

Changes:

  • Added a checkbox toggle in the settings to hide/show the search box
  • Implemented JavaScript functions to control search box visibility with localStorage persistence
  • Updated CHANGELOG.md to document this new feature

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File Description
scripts/search.js Implements the search box visibility toggle logic with event listeners and localStorage persistence
index.html Adds the "Hide Search Box" toggle UI element in settings and sets default visibility style
CHANGELOG.md Documents the new search box hide feature

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.


if (isSearchBarswitchEnabled) {
hideSearchbar();
} else if (!isShortCutSwitchEnabled) {
Copy link

Copilot AI Jan 14, 2026

Choose a reason for hiding this comment

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

Variable isShortCutSwitchEnabled is undefined. This should be isSearchBarswitchEnabled to maintain consistency with the logic for showing/hiding the search bar.

Suggested change
} else if (!isShortCutSwitchEnabled) {
} else if (!isSearchBarswitchEnabled) {

Copilot uses AI. Check for mistakes.
document.getElementById("searchbar").style.visibility = "hidden";
}

const initSearchBarswitch = (element) => {
Copy link

Copilot AI Jan 14, 2026

Choose a reason for hiding this comment

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

Function name initSearchBarswitch uses inconsistent casing. Should be initSearchBarSwitch with capital 'S' in 'Switch' to follow camelCase convention.

Copilot uses AI. Check for mistakes.
Comment on lines +431 to +434
localStorage.setItem("showSearchBarswitch", true);
} else {
showSearchbar();
localStorage.setItem("showSearchBarswitch", false);
Copy link

Copilot AI Jan 14, 2026

Choose a reason for hiding this comment

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

The localStorage key name showSearchBarswitch uses inconsistent casing. Consider renaming to showSearchBarSwitch with capital 'S' in 'Switch' for consistency.

Copilot uses AI. Check for mistakes.
Copy link
Author

Choose a reason for hiding this comment

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

@copilot open a new pull request to apply changes based on this feedback

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🤖 Fix all issues with AI agents
In `@CHANGELOG.md`:
- Line 110: The changelog entry "- Added option to hide the search box
([`@satyansh2004`]) ([`#51`])" is placed under v3.2 but should be moved to the
"[Unreleased]" section; edit CHANGELOG.md to cut that specific entry and paste
it under the existing "[Unreleased]" header (preserving the exact entry text and
attribution) so the feature is not shown as shipped in v3.2.

In `@index.html`:
- Around line 1479-1488: Add the new translation keys "hideSearchBox" and
"hideSearchBoxInfo" to all 31 locale files, following the same structure and key
naming used for similar toggles (e.g., "hideClockBox" / "hideClockBoxInfo");
populate each locale with an appropriate translation for the short label ("Hide
Search Box") and the helper text (refine the English helper to "If you don't
want the search box" or equivalent in other languages). Ensure the locale
JSON/YAML includes both keys for every language and that the UI uses these keys
(IDs hideSearchBox and hideSearchBoxInfo) so the rendered text pulls from the
locale bundles.
🧹 Nitpick comments (1)
index.html (1)

673-673: Avoid forcing visibility: visible inline (can cause a flash before persisted state is applied).
If the intent is to honor the saved “hide search box” preference on load, hard-coding visible inline works against that and can create a brief UI flicker.

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 0aaedb3 and aa77281.

📒 Files selected for processing (3)
  • CHANGELOG.md
  • index.html
  • scripts/search.js

✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.

- Added toggle to hide or show location in the location tile ([@prem-k-r](https://github.com/prem-k-r)) ([#685](https://github.com/XengShi/materialYouNewTab/pull/685))
- Added drag and drop functionality for reordering shortcuts ([@prem-k-r](https://github.com/prem-k-r)) ([#695](https://github.com/XengShi/materialYouNewTab/pull/695))
- Added edit functionality for todo list ([@GauravKukreti](https://github.com/GauravKukreti)) ([#719](https://github.com/XengShi/materialYouNewTab/pull/719))
- Added option to hide the search box ([@satyansh2004](https://github.com/satyansh2004)) ([#51](https://github.com/prem-k-r/MaterialYouNewTab/issues/51))
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Move this entry to [Unreleased] (it’s currently recorded under v3.2).
Right now the feature is documented as if it shipped on July 20, 2025, which is misleading for this PR.

Proposed change
 ## [Unreleased](https://github.com/prem-k-r/MaterialYouNewTab/compare/v3.3...main)
 
+### Added
+
+- Added option to hide the search box ([`@satyansh2004`](https://github.com/satyansh2004)) ([`#51`](https://github.com/prem-k-r/MaterialYouNewTab/issues/51))
+
 ### Improved
 ...
@@
 ## [v3.2](https://github.com/XengShi/materialYouNewTab/compare/v3...v3.2) - July 20, 2025
@@
- - Added option to hide the search box ([`@satyansh2004`](https://github.com/satyansh2004)) ([`#51`](https://github.com/prem-k-r/MaterialYouNewTab/issues/51))
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
- Added option to hide the search box ([@satyansh2004](https://github.com/satyansh2004)) ([#51](https://github.com/prem-k-r/MaterialYouNewTab/issues/51))
## [Unreleased](https://github.com/prem-k-r/MaterialYouNewTab/compare/v3.3...main)
### Added
- Added option to hide the search box ([`@satyansh2004`](https://github.com/satyansh2004)) ([`#51`](https://github.com/prem-k-r/MaterialYouNewTab/issues/51))
### Improved
...
## [v3.2](https://github.com/XengShi/materialYouNewTab/compare/v3...v3.2) - July 20, 2025
🤖 Prompt for AI Agents
In `@CHANGELOG.md` at line 110, The changelog entry "- Added option to hide the
search box ([`@satyansh2004`]) ([`#51`])" is placed under v3.2 but should be moved
to the "[Unreleased]" section; edit CHANGELOG.md to cut that specific entry and
paste it under the existing "[Unreleased]" header (preserving the exact entry
text and attribution) so the feature is not shown as shipped in v3.2.

Comment on lines +1479 to +1488
<div class="ttcont">
<div class="texts">
<div class="bigText" id="hideSearchBox">Hide Search Box</div>
<div class="infoText" id="hideSearchBoxInfo">If you do not want search box</div>
</div>
<label class="switch">
<input id="searchBoxCheckBox" type="checkbox">
<span class="toggle"></span>
</label>
</div>
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

rg -n --hidden --glob '!**/node_modules/**' -S 'hideSearchBoxInfo|hideSearchBox' .

Repository: prem-k-r/MaterialYouNewTab

Length of output: 562


🏁 Script executed:

fd -t f -e json -e js -e yml -e yaml . | grep -E '(locale|i18n|lang|translation)' | head -20

Repository: prem-k-r/MaterialYouNewTab

Length of output: 353


🏁 Script executed:

# Check the structure of a locale file
head -50 locales/en.js

Repository: prem-k-r/MaterialYouNewTab

Length of output: 1967


🏁 Script executed:

# Search for the new text strings in all locale files
rg -n 'Hide Search Box|If you do not want search box' locales/

Repository: prem-k-r/MaterialYouNewTab

Length of output: 52


🏁 Script executed:

# Count locale files
ls -1 locales/*.js | wc -l

# Show all locale files
ls -1 locales/*.js | xargs -I {} basename {}

Repository: prem-k-r/MaterialYouNewTab

Length of output: 265


🏁 Script executed:

# Check if there's a pattern for how "Hide X" settings are translated
rg -n '"hide' locales/ -i | head -20

Repository: prem-k-r/MaterialYouNewTab

Length of output: 1428


Add translations for "Hide Search Box" setting across all 31 locale files.

The IDs hideSearchBox and hideSearchBoxInfo are missing from every locale file (ar_SA, de, en, fr, ja, ko, ru, etc.). Following the established pattern for similar toggles like hideClockBox / hideClockBoxInfo, both keys need translations added to all 31 locale files.

Additionally, refine the English copy from "If you do not want search box" to "If you don't want the search box" (or similar) to match the phrasing pattern used in comparable settings.

🤖 Prompt for AI Agents
In `@index.html` around lines 1479 - 1488, Add the new translation keys
"hideSearchBox" and "hideSearchBoxInfo" to all 31 locale files, following the
same structure and key naming used for similar toggles (e.g., "hideClockBox" /
"hideClockBoxInfo"); populate each locale with an appropriate translation for
the short label ("Hide Search Box") and the helper text (refine the English
helper to "If you don't want the search box" or equivalent in other languages).
Ensure the locale JSON/YAML includes both keys for every language and that the
UI uses these keys (IDs hideSearchBox and hideSearchBoxInfo) so the rendered
text pulls from the locale bundles.

Comment on lines +419 to +456
/* ------ Event Listeners for Searchbar ------ */
const showSearchbar = () => {
document.getElementById("searchbar").style.visibility = "visible";
}

const hideSearchbar = () => {
document.getElementById("searchbar").style.visibility = "hidden";
}

const initSearchBarswitch = (element) => {
if (element.checked) {
hideSearchbar();
localStorage.setItem("showSearchBarswitch", true);
} else {
showSearchbar();
localStorage.setItem("showSearchBarswitch", false);
}
}

const hideSearchBox = document.getElementById("searchBoxCheckBox");
hideSearchBox.addEventListener("change", (e) => {
initSearchBarswitch(e.target);
});

if (localStorage.getItem("showSearchBarswitch")) {
const isSearchBarswitchEnabled = localStorage.getItem("showSearchBarswitch").toString() === "true";
document.getElementById("searchBoxCheckBox").checked = isSearchBarswitchEnabled;

if (isSearchBarswitchEnabled) {
hideSearchbar();
} else if (!isShortCutSwitchEnabled) {
showSearchbar();
}
} else {
localStorage.setItem("showSearchBarswitch", false);
}

initSearchBarswitch(hideSearchBox); No newline at end of file
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Fix ReferenceError (isShortCutSwitchEnabled) and prefer a “real hide” (hidden / display:none) over visibility.

  • Line 449: isShortCutSwitchEnabled isn’t defined in this scope; if the user has showSearchBarswitch=false, this path can crash the script.
  • UX: visibility: hidden leaves empty space and still permits focusing hidden inputs; consider searchbar.hidden = true/false (or a CSS class that sets display: none).
  • Naming: consider storing hideSearchBox=true/false to match the checkbox meaning.
Proposed fix (scoped to this block)
 /* ------ Event Listeners for Searchbar ------ */
-const showSearchbar = () => {
-    document.getElementById("searchbar").style.visibility = "visible";
-}
-
-const hideSearchbar = () => {
-    document.getElementById("searchbar").style.visibility = "hidden";
-}
+const setSearchbarHidden = (hidden) => {
+    // "hidden" collapses layout and avoids focusing invisible controls
+    searchbar.hidden = hidden;
+}
 
 const initSearchBarswitch = (element) => {
-    if (element.checked) {
-        hideSearchbar();
-        localStorage.setItem("showSearchBarswitch", true);
-    } else {
-        showSearchbar();
-        localStorage.setItem("showSearchBarswitch", false);
-    }
+    const hidden = Boolean(element.checked); // checked == hide
+    setSearchbarHidden(hidden);
+    localStorage.setItem("hideSearchBox", String(hidden));
 }
 
 const hideSearchBox = document.getElementById("searchBoxCheckBox");
 hideSearchBox.addEventListener("change", (e) => {
     initSearchBarswitch(e.target);
 });
 
-if (localStorage.getItem("showSearchBarswitch")) {
-    const isSearchBarswitchEnabled = localStorage.getItem("showSearchBarswitch").toString() === "true";
-    document.getElementById("searchBoxCheckBox").checked = isSearchBarswitchEnabled;
-
-if (isSearchBarswitchEnabled) {
-        hideSearchbar();
-    } else if (!isShortCutSwitchEnabled) {
-        showSearchbar();
-    }
-} else {
-    localStorage.setItem("showSearchBarswitch", false);
-}
+const isSearchBoxHidden = localStorage.getItem("hideSearchBox") === "true";
+hideSearchBox.checked = isSearchBoxHidden;
+setSearchbarHidden(isSearchBoxHidden);
 
 initSearchBarswitch(hideSearchBox);

@prem-k-r prem-k-r added enhancement New feature or request on-hold Blocked by another PR or pending feature. Will resume once dependency is resolved. labels Jan 27, 2026
@prem-k-r
Copy link
Owner

On hold, blocked by #103

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

Labels

enhancement New feature or request on-hold Blocked by another PR or pending feature. Will resume once dependency is resolved.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Feature]: Search box hide button

3 participants