Improve Global Searching in admin panel#2327
Conversation
📝 WalkthroughWalkthroughThe PR integrates a third-party global search modal plugin by adding the Composer dependency, registering it in the admin panel provider, bundling compiled Tailwind CSS styling with theme variables and utilities, and implementing JavaScript modules for localStorage-backed search history/favorites management and DOM event-driven modal interaction. ChangesGlobal Search Modal Feature
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
public/js/charrafimed/global-search-modal/components/global-search-modal-observer.js (3)
25-28: 💤 Low valueConsider removing redundant keypress handler.
The
keypressevent listener is redundant sincekeydown(registered at line 18) already covers all keyboard input including character keys. Thekeypressevent is also a legacy event that's been deprecated in favor ofkeydownandbeforeinput.♻️ Proposed fix
inputElement.setAttribute("readonly", true); inputElement.setAttribute("tabindex", "-1"); } }, - inputElement.addEventListener("keypress", (event) => { - event.preventDefault(); - event.stopPropagation(); - }, true); - inputElement.setAttribute("readonly", true);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@public/js/charrafimed/global-search-modal/components/global-search-modal-observer.js` around lines 25 - 28, Remove the redundant "keypress" listener on inputElement: locate the inputElement.addEventListener("keypress", ...) block and delete it, leaving the existing "keydown" listener (registered earlier) to handle keyboard input; ensure no other code depends on the keypress handler before removal and keep event.preventDefault()/stopPropagation() behavior in the "keydown" handler if needed.
57-57: 💤 Low valuePass string value to setAttribute.
setAttributeexpects string values. While the number0will be coerced to the string"0", it's better to be explicit and pass the string directly for consistency.♻️ Proposed fix
inputElement.disabled = false; inputElement.readOnly = false; - inputElement.setAttribute("tabindex", 0); + inputElement.setAttribute("tabindex", "0"); } });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@public/js/charrafimed/global-search-modal/components/global-search-modal-observer.js` at line 57, The call inputElement.setAttribute("tabindex", 0) passes a number; change it to pass a string by using "0" so setAttribute receives a string value (update the setAttribute invocation on inputElement to use "0" instead of 0).
4-4: 💤 Low valueRemove unused property.
The
observerproperty is declared but never used in this module. Consider removing it to keep the code clean.♻️ Proposed fix
function observer() { return { - observer: null, modalOpen: false, init: function() {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@public/js/charrafimed/global-search-modal/components/global-search-modal-observer.js` at line 4, The exported object in global-search-modal-observer.js declares an unused property named "observer"; remove the "observer" property declaration from the object (and any related/commented code in that module) so the module no longer contains an unused symbol, and run a quick grep for "observer" in that file to confirm there are no remaining references before committing.public/css/charrafimed/global-search-modal/global-search-modal.css (1)
1-1215: ⚡ Quick winConsider excluding compiled CSS from linting.
This file is compiled Tailwind CSS output (as indicated by the header comment). Stylelint violations in generated/compiled files are expected and should not be manually fixed, as changes would be overwritten on rebuild.
Consider adding this file to your
.stylelintignoreor similar linting configuration to suppress false positives.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@public/css/charrafimed/global-search-modal/global-search-modal.css` around lines 1 - 1215, This is generated Tailwind output (see header "/*! tailwindcss v4.1.12" and large compiled rules like :root, :host), so instead of editing it, add public/css/charrafimed/global-search-modal/global-search-modal.css to the stylelint ignore list (or update your lint config to exclude compiled CSS directories) or add an appropriate generated-file ignore rule; alternatively, if you prefer a per-file suppression, prepend a stylelint disable directive to the generated file as part of your build step so lint rules no longer run on this compiled output.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@public/js/charrafimed/global-search-modal/components/global-search-modal-observer.js`:
- Line 29: The code incorrectly uses inputElement.setAttribute("readonly", true)
and later sets readonly to "false" in listenForModalClose; change these to use
the DOM property or proper boolean attribute handling: replace
setAttribute("readonly", true) with inputElement.readOnly = true, and replace
the code that sets readonly to false (in listenForModalClose) with
inputElement.readOnly = false (or use removeAttribute("readonly") if you prefer
attribute manipulation) so the readonly state is toggled correctly without
creating readonly="false".
In
`@public/js/charrafimed/global-search-modal/components/global-search-modal-search.js`:
- Around line 23-24: The getLocalStorage method currently calls
localStorage.getItem and JSON.parse directly and can throw in private mode,
quota/security errors, or on invalid JSON; wrap the localStorage.getItem +
JSON.parse sequence in a try-catch inside getLocalStorage, catch any exception,
optionally log the error, and return an empty array as the safe fallback so
callers of getLocalStorage (refer to getLocalStorage) never receive a thrown
exception or undefined.
---
Nitpick comments:
In `@public/css/charrafimed/global-search-modal/global-search-modal.css`:
- Around line 1-1215: This is generated Tailwind output (see header "/*!
tailwindcss v4.1.12" and large compiled rules like :root, :host), so instead of
editing it, add
public/css/charrafimed/global-search-modal/global-search-modal.css to the
stylelint ignore list (or update your lint config to exclude compiled CSS
directories) or add an appropriate generated-file ignore rule; alternatively, if
you prefer a per-file suppression, prepend a stylelint disable directive to the
generated file as part of your build step so lint rules no longer run on this
compiled output.
In
`@public/js/charrafimed/global-search-modal/components/global-search-modal-observer.js`:
- Around line 25-28: Remove the redundant "keypress" listener on inputElement:
locate the inputElement.addEventListener("keypress", ...) block and delete it,
leaving the existing "keydown" listener (registered earlier) to handle keyboard
input; ensure no other code depends on the keypress handler before removal and
keep event.preventDefault()/stopPropagation() behavior in the "keydown" handler
if needed.
- Line 57: The call inputElement.setAttribute("tabindex", 0) passes a number;
change it to pass a string by using "0" so setAttribute receives a string value
(update the setAttribute invocation on inputElement to use "0" instead of 0).
- Line 4: The exported object in global-search-modal-observer.js declares an
unused property named "observer"; remove the "observer" property declaration
from the object (and any related/commented code in that module) so the module no
longer contains an unused symbol, and run a quick grep for "observer" in that
file to confirm there are no remaining references before committing.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: bac01114-6adb-405b-8d21-2e9b36136975
⛔ Files ignored due to path filters (1)
composer.lockis excluded by!**/*.lock
📒 Files selected for processing (5)
app/Providers/Filament/AdminPanelProvider.phpcomposer.jsonpublic/css/charrafimed/global-search-modal/global-search-modal.csspublic/js/charrafimed/global-search-modal/components/global-search-modal-observer.jspublic/js/charrafimed/global-search-modal/components/global-search-modal-search.js
Added filament global search plugin to improve on filaments stock global search.