-
Notifications
You must be signed in to change notification settings - Fork 44
Search page #834
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
Search page #834
Conversation
|
📦 build.zip [updated at Sep 5, 1:23:50 PM UTC] |
| position: 'relative', | ||
| left: -16, | ||
| width: 'calc(100% + 32px)', | ||
| overflow: 'hidden', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This fixes horizontal scroll appear because of the animated point on the chart
848567e to
eb17b98
Compare
src/background/Wallet/model/types.ts
Outdated
| configurableNonce?: boolean; | ||
| invitationBannerDismissed?: boolean; | ||
| recentAddresses?: string[]; | ||
| recentSearch?: string[]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This shouldn't be part of wallet record; instead it should be a separate storage entry, like chain-configs
| onClick={() => { | ||
| walletPort.request('assetClicked', { | ||
| assetId: position.asset.id, | ||
| pathname, | ||
| section: 'Overview', | ||
| }); | ||
| }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What if asset link is pressed using the keyboard?
Ideally "screen view" event should be enough for this. Maybe these links should have additional search params to let analytics know that they have been visited from search results.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe this is an approach for analytics to track all usage of the search page.
However, the previous screen name really should be enough for this. I will ask about this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See comments
5f77365 to
e37fd56
Compare
e37fd56 to
4c0b443
Compare
Uh oh!
There was an error while loading. Please reload this page.