Fixed inconsistent UI of clear search icon across all search bars#2866
Fixed inconsistent UI of clear search icon across all search bars#2866kasya merged 11 commits intoOWASP:mainfrom
Conversation
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings. WalkthroughUpdated Tailwind CSS classes for the clear-search buttons in two frontend components and their unit test; only styling/className changes were made (dimensions, hover/focus and dark-mode styles). No functional, event-handling, or exported API changes. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used🧠 Learnings (3)📓 Common learnings📚 Learning: 2025-09-17T02:42:41.928ZApplied to files:
📚 Learning: 2025-11-17T16:47:05.578ZApplied to files:
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
🔇 Additional comments (2)
Tip ✨ Issue Enrichment is now available for GitHub issues!CodeRabbit can now help you manage issues more effectively:
Disable automatic issue enrichmentTo disable automatic issue enrichment, add the following to your issue_enrichment:
auto_enrich:
enabled: falseThanks 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
frontend/src/components/MultiSearch.tsx (1)
244-253: Align clear button hover + focus styles withSearchBarfor consistencyThis update fixes the icon-visibility issue (hover background + text color look fine in both themes), but the clear button here now behaves slightly differently from the one in
Search.tsx:
- Different hover palette (
hover:bg-gray-600 hover:text-gray-200vshover:bg-gray-800 dark:hover:bg-gray-400 hover:text-gray-100 dark:hover:text-gray-800).- No explicit focus styles here, while
Search.tsxadds a focus ring.Given the PR goal of consistent clear-search UI across all search bars, it’s worth standardizing both the hover palette and focus treatment. For example, you could mirror the
SearchBarbutton styles:- <button - type="button" - className="absolute top-1/2 right-2 -translate-y-1/2 rounded-full p-1 w-8 h-8 text-gray-400 - hover:bg-gray-600 - hover:text-gray-200" + <button + type="button" + className="absolute top-1/2 right-2 -translate-y-1/2 rounded-full p-1 w-8 h-8 text-gray-400 + focus:ring-2 focus:ring-gray-300 focus:outline-hidden + hover:bg-gray-800 dark:hover:bg-gray-400 hover:text-gray-100 dark:hover:text-gray-800" onClick={handleClearSearch} aria-label="Clear search" >If design intentionally wants slightly different palettes between these contexts, I’d still recommend at least adding the focus ring here for keyboard/a11y parity.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
frontend/src/components/MultiSearch.tsx(1 hunks)frontend/src/components/Search.tsx(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: Rajgupta36
Repo: OWASP/Nest PR: 2288
File: frontend/src/components/ActionButton.tsx:0-0
Timestamp: 2025-09-17T02:42:41.928Z
Learning: In frontend/src/components/ActionButton.tsx, the user Rajgupta36 intentionally changed text-blue-600 to text-[#1D7BD7] to align the text color with the border color (#1D7BD7) for visual consistency, prioritizing design alignment over theme tokens.
📚 Learning: 2025-09-17T02:42:41.928Z
Learnt from: Rajgupta36
Repo: OWASP/Nest PR: 2288
File: frontend/src/components/ActionButton.tsx:0-0
Timestamp: 2025-09-17T02:42:41.928Z
Learning: In frontend/src/components/ActionButton.tsx, the user Rajgupta36 intentionally changed text-blue-600 to text-[#1D7BD7] to align the text color with the border color (#1D7BD7) for visual consistency, prioritizing design alignment over theme tokens.
Applied to files:
frontend/src/components/Search.tsxfrontend/src/components/MultiSearch.tsx
🔇 Additional comments (1)
frontend/src/components/Search.tsx (1)
88-97: Clear button hover/focus behavior looks correct and fixes the visibility bugThe updated classes give the clear button a consistent, accessible behavior:
- Fixed size (
w-8 h-8) keeps the hit area predictable.- Default
text-gray-400and the hover pairs (hover:bg-gray-800+hover:text-gray-100,dark:hover:bg-gray-400+dark:hover:text-gray-800) ensure good contrast in both light and dark themes.- Added
focus:ring-2 focus:ring-gray-300 focus:outline-hiddenprovides a clear keyboard focus indicator.Click behavior and ARIA labeling are unchanged, so functionality is preserved while fixing the visual inconsistency described in the issue.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
frontend/src/components/MultiSearch.tsx(1 hunks)frontend/src/components/Search.tsx(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-17T02:42:41.928Z
Learnt from: Rajgupta36
Repo: OWASP/Nest PR: 2288
File: frontend/src/components/ActionButton.tsx:0-0
Timestamp: 2025-09-17T02:42:41.928Z
Learning: In frontend/src/components/ActionButton.tsx, the user Rajgupta36 intentionally changed text-blue-600 to text-[#1D7BD7] to align the text color with the border color (#1D7BD7) for visual consistency, prioritizing design alignment over theme tokens.
Applied to files:
frontend/src/components/MultiSearch.tsxfrontend/src/components/Search.tsx
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Run frontend checks
- GitHub Check: CodeQL (python)
- GitHub Check: CodeQL (javascript-typescript)
🔇 Additional comments (1)
frontend/src/components/Search.tsx (1)
90-90: Verify WCAG contrast compliance for light mode hover state.The explicit dimensions (
h-8 w-8) and focus ring are good additions. However, the hover styling differs from the suggested fix in issue #2849. Your implementation useshover:bg-gray-400withhover:text-gray-200for light mode.Gray text (#E5E7EB) on a gray background (#9CA3AF) may not meet the WCAG AA contrast requirement of 3:1 for UI components. The dark mode state (
dark:hover:bg-gray-600) should have sufficient contrast, but please verify the light mode hover colors meet accessibility standards.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
frontend/src/components/Search.tsx(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-17T02:42:41.928Z
Learnt from: Rajgupta36
Repo: OWASP/Nest PR: 2288
File: frontend/src/components/ActionButton.tsx:0-0
Timestamp: 2025-09-17T02:42:41.928Z
Learning: In frontend/src/components/ActionButton.tsx, the user Rajgupta36 intentionally changed text-blue-600 to text-[#1D7BD7] to align the text color with the border color (#1D7BD7) for visual consistency, prioritizing design alignment over theme tokens.
Applied to files:
frontend/src/components/Search.tsx
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: CodeQL (javascript-typescript)
- GitHub Check: CodeQL (python)
- GitHub Check: Run frontend checks
- GitHub Check: Run spell check
- GitHub Check: Run pre-commit checks
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
frontend/src/components/MultiSearch.tsx (1)
246-246: LGTM! Clear-search icon hover issue resolved.The className updates successfully fix the original issue where the clear-search icon was invisible on hover. The new hover states (
hover:text-gray-200onhover:bg-gray-400) ensure the icon remains visible in light mode, anddark:hover:bg-gray-600handles dark mode appropriately. Focus accessibility is properly addressed with the focus ring classes.Optional: Add explicit dark mode focus ring for consistency.
The input field at line 241 specifies
dark:focus:ring-blue-300, but the button only hasfocus:ring-gray-300(which applies to both light and dark modes). Consider addingdark:focus:ring-gray-400for a more visible focus indicator in dark mode:- className="absolute top-1/2 right-2 h-8 w-8 -translate-y-1/2 rounded-full p-1 text-gray-400 hover:bg-gray-400 hover:text-gray-200 focus:ring-2 focus:ring-gray-300 focus:outline-hidden dark:hover:bg-gray-600" + className="absolute top-1/2 right-2 h-8 w-8 -translate-y-1/2 rounded-full p-1 text-gray-400 hover:bg-gray-400 hover:text-gray-200 focus:ring-2 focus:ring-gray-300 focus:outline-hidden dark:hover:bg-gray-600 dark:focus:ring-gray-400"frontend/src/components/Search.tsx (1)
90-90: LGTM! Consistent styling across search components.The className updates mirror the changes in MultiSearch.tsx, ensuring a consistent clear-search button appearance and behavior across the codebase. The hover state fix (
hover:text-gray-200onhover:bg-gray-400) resolves the visibility issue described in issue #2849, and focus states provide proper keyboard accessibility.Optional: Consider explicit dark mode focus ring.
For enhanced visibility and consistency with the input field's dark mode styling at line 85, consider adding
dark:focus:ring-gray-400:- className="absolute top-1/2 right-2 h-8 w-8 -translate-y-1/2 rounded-full p-1 text-gray-400 hover:bg-gray-400 hover:text-gray-200 focus:ring-2 focus:ring-gray-300 focus:outline-hidden dark:hover:bg-gray-600" + className="absolute top-1/2 right-2 h-8 w-8 -translate-y-1/2 rounded-full p-1 text-gray-400 hover:bg-gray-400 hover:text-gray-200 focus:ring-2 focus:ring-gray-300 focus:outline-hidden dark:hover:bg-gray-600 dark:focus:ring-gray-400"
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
frontend/src/components/MultiSearch.tsx(1 hunks)frontend/src/components/Search.tsx(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-09-17T02:42:41.928Z
Learnt from: Rajgupta36
Repo: OWASP/Nest PR: 2288
File: frontend/src/components/ActionButton.tsx:0-0
Timestamp: 2025-09-17T02:42:41.928Z
Learning: In frontend/src/components/ActionButton.tsx, the user Rajgupta36 intentionally changed text-blue-600 to text-[#1D7BD7] to align the text color with the border color (#1D7BD7) for visual consistency, prioritizing design alignment over theme tokens.
Applied to files:
frontend/src/components/MultiSearch.tsxfrontend/src/components/Search.tsx
📚 Learning: 2025-04-30T13:41:20.846Z
Learnt from: codic-yeeshu
Repo: OWASP/Nest PR: 1444
File: frontend/src/components/NavDropDown.tsx:0-0
Timestamp: 2025-04-30T13:41:20.846Z
Learning: When implementing dropdown menus or similar interactive components, always include proper accessibility features: ARIA attributes (aria-expanded, aria-haspopup, aria-controls), keyboard navigation support (Enter, Space, Escape keys), and mechanisms to close dropdowns when clicking outside.
Applied to files:
frontend/src/components/Search.tsx
|
I just saw you that you also edited the frontend tests. I made a mistake there before pushing the PR I did ran make check but it was on the main branch that's why it didn't caused any error. Really sorry for any inconvenience this caused. |
|
@arkid15r can you look into this review , wasn't able to merge it because by the time i tried the main branch had new commits and i had to update the branch again. |
|
kasya
left a comment
kasya
left a comment
There was a problem hiding this comment.
@anirudhprmar thanks for working on this! 👍🏼
2 participants
Proposed change
Resolves #2849
This PR fixed inconsistencies in UI of clear search icon across all the search bars which support both themes.
changes made:
for home page:
dark mode:
light mode:
for projects and contributions page:
dark mode:
light mode:
Checklist
make check-testlocally and all tests passed