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

Fix oasis address links even when toggled to ETH addresses #1329

Merged
merged 2 commits into from
Mar 19, 2024

Conversation

lukaw3d
Copy link
Member

@lukaw3d lukaw3d commented Mar 15, 2024

Makes "plain" links still link to the address. User might not have known they needed to toggle the address format before they could open a "plain" address.

Fixes #1220: oasis addresses in e.g. consensus deposit transactions in EVM runtimes were not clickable in mobile vertical layout of transactions because it is implicitly toggled to ETH addresses (and can't be toggled to Oasis).

Before After
localhost_1234_mainnet_sapphire_tx_2c997a87b4f57c0266a780cab8bb1f39b35d37d6c894c79e2a43395b3323ad85 (1) image
localhost_1234_mainnet_sapphire_tx_2c997a87b4f57c0266a780cab8bb1f39b35d37d6c894c79e2a43395b3323ad85 (3) image

Copy link

github-actions bot commented Mar 15, 2024

Deployed to Cloudflare Pages

Latest commit: 243c96f59b3f3a7bbc998e9a0d50dc1b875c19ef
Status:✅ Deploy successful!
Preview URL: https://7b3eeab7.oasis-explorer.pages.dev

) : (
<Link component={RouterLink} to={to}>
<Link component={RouterLink} to={to} sx={{ fontWeight: plain ? 400 : undefined }}>
Copy link
Contributor

@buberdds buberdds Mar 18, 2024

Choose a reason for hiding this comment

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

all links should be 700 according to Don (same for TransactionLink). If that is true, why we even need plain?

I am a bit confused what plain is doing now (in master not this PR). We render (...)Link component and we use that prop to not render a link?

Copy link
Member Author

Choose a reason for hiding this comment

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

all links should be 700 according to Don (same for TransactionLink). If that is true, why we even need plain?

fixed

I am a bit confused what plain is doing now (in master not this PR). We render (...)Link component and we use that prop to not render a link?

in most cases it displayed just text instead of a link, I guess so that toggling to ETH addresses would not contain confusing oasis1 addresses 🤷

@lukaw3d lukaw3d force-pushed the lw/fix-oasis-links branch from d0c547f to 58ebf20 Compare March 19, 2024 15:40
lukaw3d added 2 commits March 19, 2024 16:49
Makes "plain" links still link to the address. User might not have known they
needed to toggle the address format before they could open a "plain" address.

Fixes #1220: oasis addresses in e.g. consensus deposit transactions in EVM
runtimes were not clickable in mobile vertical layout of transactions because
it is implicitly toggled to ETH addresses (and can't be toggled to Oasis).
@lukaw3d lukaw3d force-pushed the lw/fix-oasis-links branch from 58ebf20 to 243c96f Compare March 19, 2024 15:49
@lukaw3d lukaw3d merged commit 7dc2a0e into master Mar 19, 2024
8 checks passed
@lukaw3d lukaw3d deleted the lw/fix-oasis-links branch March 19, 2024 18:16
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.

[Bug]: Can't open non-EVM transactions from vertical list of transactions
2 participants