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

feat: translate known addresses #573

Merged
merged 8 commits into from
Nov 23, 2023
Merged

Conversation

janmichek
Copy link
Collaborator

@janmichek janmichek commented Nov 14, 2023

Description

resolves #451

known addresses are listed here https://github.com/aeternity/aescan/pull/573/files#diff-bd2e652035adaf33b6125f14a823cf76feae0c1c1a592e9a6c14a14951a52a28R96

Demo

firefox_LhuLGjtRPx.mp4

Checklist:

Copy link

Deployed to https://pr-573-aescan.stg.aepps.com

Copy link
Collaborator

@michele-franchi michele-franchi left a comment

Choose a reason for hiding this comment

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

Looks good, we could also display it in the account detail but it's already a good starting point.

src/utils/constants.js Outdated Show resolved Hide resolved
janmichek and others added 2 commits November 16, 2023 16:37
Co-authored-by: Michele F. <michele-franchi@users.noreply.github.com>
@janmichek
Copy link
Collaborator Author

Looks good, we could also display it in the account detail but it's already a good starting point.

incorporated to account detail. Hope that's how you imagined it .)

<copy-chip
:label="formatEllipseHash(accountDetails.id)"
:label="formatKnownAddress(accountDetails.id)"
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I touched this and noticed the usage of styling classes for the different views, which I recently replaced. Here was forgotten, so I adjusted it also in sibling files

Copy link
Collaborator

@Liubov-crypto Liubov-crypto left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Collaborator

@michele-franchi michele-franchi left a comment

Choose a reason for hiding this comment

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

It looks good but I would not apply it in the account detail like this, I have some other ideas on how to do it and I'll open a followup.
Please remove the AccountDetailsPanel implementation and I'll approve.

@janmichek
Copy link
Collaborator Author

It looks good but I would not apply it in the account detail like this, I have some other ideas on how to do it and I'll open a followup. Please remove the AccountDetailsPanel implementation and I'll approve.

Removed

Copy link
Collaborator

@michele-franchi michele-franchi left a comment

Choose a reason for hiding this comment

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

Good job

@janmichek janmichek merged commit 2bda957 into develop Nov 23, 2023
1 of 2 checks passed
@janmichek janmichek deleted the Show-names-for-known-addresse branch November 23, 2023 11:13
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.

Show names for known addresses
3 participants