-
Notifications
You must be signed in to change notification settings - Fork 6
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(btc-link): link to the different btc env #412
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
src/components/Link/index.tsx
Outdated
}) | ||
|
||
return ( | ||
<Link | ||
lng={lng ?? (locale as 'en' | 'zh')} | ||
ref={ref} | ||
{...props} | ||
to={`${config.BITCOIN_EXPLORER}${IS_MAINNET ? '' : `/${identity}`}${path}/${id}${anchor ? `#${anchor}` : ''}`} | ||
to={`${config.BITCOIN_EXPLORER}${IS_MAINNET ? '' : `/${identity ?? 'testnet'}`}${path}/${address ?? id}${ |
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.
It's better to assign the fallback default value where identity
is set so the default value won't be missed elsewhere.
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.
It's better to assign the fallback default value where
identity
is set so the default value won't be missed elsewhere.
This is a basic component, it won't pass the identity to any other components, so I think it is not necessary, here the default value obviously is the "testnet".
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.
We have to remember ?? 'testnet'
when identity
is used elsewhere in this component.
If identity
was set with a default testnet
when it's defined, for example, at https://github.com/Magickbase/ckb-explorer-frontend/pull/412/files/603e800939a0482a24b285853666d9d22bd0454f#diff-f4edd6698ac8ef92e05ed39a633d29fd9c35ac226d25bebf00eb6d95fd8f1214L58, we don't have to remember this detail.
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.
We have to remember
?? 'testnet'
whenidentity
is used elsewhere in this component. Ifidentity
was set with a defaulttestnet
when it's defined, for example, at https://github.com/Magickbase/ckb-explorer-frontend/pull/412/files/603e800939a0482a24b285853666d9d22bd0454f#diff-f4edd6698ac8ef92e05ed39a633d29fd9c35ac226d25bebf00eb6d95fd8f1214L58, we don't have to remember this detail.
It might be null, and why to remember this, I didn't understand.
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.
We have to remember
?? 'testnet'
whenidentity
is used elsewhere in this component. Ifidentity
was set with a defaulttestnet
when it's defined, for example, at Magickbase/ckb-explorer-frontend/pull/412/files/603e800939a0482a24b285853666d9d22bd0454f#diff-f4edd6698ac8ef92e05ed39a633d29fd9c35ac226d25bebf00eb6d95fd8f1214L58, we don't have to remember this detail.It might be null, and why to remember this, I didn't understand.
When other programmers add codes to this component, they should pay attention to use identity
with ?? 'testnet'
to keep all references consistent.
Say we are going to add a mark when it's on testnet, <badge>{identity ?? 'testnet'}</badge>
should be added, in the case that the coder has noticed it.
If the coder is reckless, he/she may add <badge>{identity}</badge>
directly, which is different from identity ?? 'testnet'
If the identity
is defined with a default value 'testnet'
, <badge>{identity}</badge>
can be used directly.
Magickbase/ckb-explorer-public-issues#747