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

[explorer] Adds Search Filter #1959

Merged
merged 7 commits into from
May 13, 2022
Merged

[explorer] Adds Search Filter #1959

merged 7 commits into from
May 13, 2022

Conversation

apburnie
Copy link
Contributor

Fixes Issue #1871 through the use of a dropdown box. The UI has been optimised bearing in mind screens even as small as the Galaxy Fold.

image

image

@apburnie apburnie linked an issue May 13, 2022 that may be closed by this pull request
@apburnie apburnie requested review from stella3d, 666lcz and Jibz1 May 13, 2022 18:44
@apburnie apburnie changed the title Adds Search Filter [explorer] Adds Search Filter May 13, 2022
@666lcz
Copy link
Contributor

666lcz commented May 13, 2022

Quick comment: is there a regression in the padding for "search by ID" or are you in a really old branch? Below is what's in production

CleanShot 2022-05-13 at 11 47 41

@stella3d
Copy link
Contributor

Added a small PR that changes the placeholder text for address and transaction categories, as shown here.

Also adds a TS type for defining the correct strings for categories

image

image

Copy link
Contributor

@stella3d stella3d left a comment

Choose a reason for hiding this comment

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

tested this locally, works well, code looks good. great job!

@stella3d stella3d enabled auto-merge (squash) May 13, 2022 19:15
@666lcz 666lcz disabled auto-merge May 13, 2022 19:16
@666lcz
Copy link
Contributor

666lcz commented May 13, 2022

Please make sure fixing the padding issue before merging(see comment above)

case 'addresses':
return 'Search by address';
case 'transactions':
return 'Search by digest';
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
return 'Search by digest';
return 'Search by transaction id';

Even though "digest" is technically more accurate, we use TxnId everywhere else, so let's be consistent here
CleanShot 2022-05-13 at 12 18 15

@stella3d stella3d merged commit 46032fa into main May 13, 2022
@stella3d stella3d deleted the andrew/SearchFilter branch May 13, 2022 19:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[explorer] Search Filter
3 participants