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

Highlight text matches search results #646

Merged
merged 8 commits into from
Feb 2, 2024

Conversation

csillag
Copy link
Contributor

@csillag csillag commented Jul 3, 2023

We can do this in the search results (in the example, we are searching for "gen"):

image

@github-actions
Copy link

github-actions bot commented Jul 3, 2023

Deployed to Cloudflare Pages

Latest commit: c43f93619ae2f643166a23ac222398fe32e0f490
Status:✅ Deploy successful!
Preview URL: https://a9219a76.oasis-explorer.pages.dev

@csillag csillag force-pushed the csillag/highlight-matches-in-toke-search-results branch 2 times, most recently from eb0387d to 480cfd5 Compare July 9, 2023 14:35
@csillag csillag changed the title Highlight matches in toke search results Highlight matches in token search results Jul 9, 2023
@csillag csillag force-pushed the csillag/highlight-matches-in-toke-search-results branch 7 times, most recently from c2445fe to 04a7277 Compare July 11, 2023 06:34
@csillag csillag force-pushed the csillag/highlight-matches-in-toke-search-results branch from 04a7277 to 223c032 Compare January 27, 2024 12:40
@csillag
Copy link
Contributor Author

csillag commented Jan 27, 2024

Friendly reminder: this is still waiting for input from @donouwens

@csillag csillag force-pushed the csillag/highlight-matches-in-toke-search-results branch from 223c032 to ce67ff3 Compare January 27, 2024 12:48
@csillag csillag changed the title Highlight matches in token search results Highlight text matches search results Jan 28, 2024
@donouwens
Copy link
Collaborator

Yes, please let's move forward with this. I would like to change the design a bit though:

Can we not change the text color, but rather add a highlight marker/background, like in most common text highlights when searching, please?

The color would be #FFFF54 - here's the visual: https://www.figma.com/file/8dwSyYOOm3vlgbvxPGT29p/Block-Explorer-(Post-Launch)?type=design&node-id=9009%3A89887&mode=design&t=gK1wSzYZ5GupHmBP-1

@csillag
Copy link
Contributor Author

csillag commented Jan 29, 2024

Can we not change the text color, but rather add a highlight marker/background, like in most common text highlights when searching, please?

The color would be #FFFF54

Done. (See image above)

Thanks. Please note that we don't (and currently can't) do free next search inside hashes, only on token names and token symbols. (And hopefully soon in proposal names.) So that part fo the visual doesn't match reality.

@csillag
Copy link
Contributor Author

csillag commented Jan 29, 2024

Now this has the blessing of @donouwens , so ready for review!

@csillag csillag marked this pull request as ready for review January 29, 2024 14:52
@csillag csillag force-pushed the csillag/highlight-matches-in-toke-search-results branch 4 times, most recently from 9d8242b to 8f9496f Compare January 30, 2024 10:05
@csillag csillag requested review from lukaw3d and buberdds January 30, 2024 14:44
@csillag csillag force-pushed the csillag/highlight-matches-in-toke-search-results branch from debacb1 to 443c135 Compare January 30, 2024 16:26
@csillag
Copy link
Contributor Author

csillag commented Jan 30, 2024

Now that proposal search is merged, I also added support for highlighting the matches in those results.

@csillag csillag force-pushed the csillag/highlight-matches-in-toke-search-results branch 4 times, most recently from 705e4de to 7463470 Compare February 1, 2024 11:21
@csillag csillag force-pushed the csillag/highlight-matches-in-toke-search-results branch 2 times, most recently from ddd39fd to de03474 Compare February 1, 2024 18:36
src/app/components/HighlightedText/text-matching.ts Outdated Show resolved Hide resolved
src/app/components/HighlightedText/text-matching.ts Outdated Show resolved Hide resolved
src/app/components/HighlightedText/index.tsx Outdated Show resolved Hide resolved
src/app/components/HighlightedText/index.tsx Outdated Show resolved Hide resolved
src/app/components/HighlightedText/text-matching.ts Outdated Show resolved Hide resolved
src/app/components/HighlightedText/text-matching.ts Outdated Show resolved Hide resolved
src/app/components/HighlightedText/text-matching.ts Outdated Show resolved Hide resolved
src/app/components/HighlightedText/text-matching.ts Outdated Show resolved Hide resolved
@@ -49,7 +54,7 @@ export const TokenTitleCard: FC<{ scope: SearchScope; address: string }> = ({ sc
flexWrap: 'wrap',
}}
>
{token?.name ?? t('common.missing')}
{token?.name ? <HighlightedText text={token.name} pattern={searchTerm} /> : t('common.missing')}
Copy link
Member

Choose a reason for hiding this comment

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

ah, passing searchTerm through all the layers is painful. Can later refactor so HighlightedText extracts it from URL.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Indeed it's a bit of a pain. I'm not sure about the refactoring though, because there are use cases when there are multiple different highlighted terms active at the same time at the same page, coming from different sources..

@csillag csillag force-pushed the csillag/highlight-matches-in-toke-search-results branch 5 times, most recently from 1dddf40 to 5b9aacf Compare February 2, 2024 03:04
@csillag csillag force-pushed the csillag/highlight-matches-in-toke-search-results branch from 5b9aacf to c43f936 Compare February 2, 2024 03:07
@csillag csillag enabled auto-merge February 2, 2024 03:08
@csillag csillag merged commit 2aa9cef into master Feb 2, 2024
8 checks passed
@csillag csillag deleted the csillag/highlight-matches-in-toke-search-results branch February 2, 2024 03:09
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.

4 participants