Skip to content
This repository was archived by the owner on Jan 11, 2023. It is now read-only.

Fix searchInput UI when there are no search results - #7608

Merged
darkwing merged 9 commits into
firefox-devtools:masterfrom
nagpalm7:search
Jan 2, 2019
Merged

Fix searchInput UI when there are no search results#7608
darkwing merged 9 commits into
firefox-devtools:masterfrom
nagpalm7:search

Conversation

@nagpalm7

Copy link
Copy Markdown
Contributor

Fixes #7325

Summary of Changes

  • Remove the sad face emoji that appears when no match are there for corresponding search
  • Change the bright orange color to grey50
  • Change "No results" string to "No results found"

Test Plan

Search for any random keyword in any file of a project, which is most likely to be not found .

Screenshots

screenshot from 2018-12-29 15-06-13

@nagpalm7
nagpalm7 requested a review from flodolo as a code owner December 29, 2018 09:39
Comment thread assets/panel/debugger.properties Outdated
# LOCALIZATION NOTE (editor.noResults): Editor Search bar message
# for when no results found.
editor.noResults=No results
editor.noResults=No results found

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This type of changes requires a new string ID
https://developer.mozilla.org/en-US/docs/Making_String_Changes

Make sure to also update the reference in the comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

If your changes are relevant only for English — for example, to correct a typographical error or to make letter case consistent — then there is generally no need to update the entity name.

I just appended "found" keyword which doesn't change the meaning still if it requires new string ID let me know. @flodolo

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

It might not change the general meaning, but the resulting translations will be likely different. I'd still change the ID, if it's important to update the string for en-US

@nagpalm7 nagpalm7 Dec 29, 2018

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Add new string with id : "editor.noResultsFound" and value : "No results found"

Comment thread assets/panel/debugger.properties Outdated
# LOCALIZATION NOTE (editor.noResults): Editor Search bar message
# for when no results found.
editor.noResults=No results found
editor.noResults=No results

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Is this string still used after your changes? If it isn't, you should remove it from the file entirely, and update the localization note to reference the new string instead.

@flodolo flodolo left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Thanks, l10n bits look good. Leaving to devs the actual review ;-)

Comment thread src/components/shared/SearchInput.css Outdated

.search-field input.empty {
color: var(--theme-highlight-orange);
color: #737373;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

is there a variable color that we could use? something that starts with --theme-

https://developer.mozilla.org/en-US/docs/Tools/DevToolsColors

@nagpalm7 nagpalm7 Dec 31, 2018

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes I have made the changes. @jasonLaster

@darkwing

darkwing commented Jan 2, 2019

Copy link
Copy Markdown
Contributor

Added a small change to make the result text not selectable

@darkwing
darkwing merged commit faf2372 into firefox-devtools:master Jan 2, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants