Skip to content
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

Fix advanced search options (missing Fluent strings, CSS) #3508

Merged
merged 4 commits into from
Jan 6, 2025

Conversation

flodolo
Copy link
Collaborator

@flodolo flodolo commented Jan 4, 2025

For fa-w, see #3507

The console is full or React errors when opening the panel. The edit removed a bunch of trailing whitespaces, which I think it's useful to remove anyway.

@flodolo flodolo requested a review from mathjazz January 4, 2025 06:16
@flodolo flodolo changed the title Fix search options panel (missing Fluent strings, unused class) Fix advanced search options (missing Fluent strings, CSS) Jan 4, 2025
@@ -38,7 +38,7 @@ const SearchOption = ({
onToggle();
}}
>
<i className='fa fa-w'></i>
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Was this meant to be fa-fw (fixed-width)? In any case, the menu seems to work fine without it.

@mathjazz
Copy link
Collaborator

mathjazz commented Jan 6, 2025

Thanks for the update!

Didn't notice this before - I don't get a hover effect on the Apply search options button - neither the text color nor the mouse cursor change.

@flodolo
Copy link
Collaborator Author

flodolo commented Jan 6, 2025

I think I've fixed it, but I'm not sure I understand the intent of the original code (why was the margin-top changing? Was the extra class ever used?)

@mathjazz mathjazz merged commit 6bab0c9 into mozilla:main Jan 6, 2025
5 checks passed
@flodolo flodolo deleted the fix_nits_search branch January 6, 2025 14:00
haodave pushed a commit to ScanTrust/pontoon that referenced this pull request Jan 14, 2025
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.

2 participants