-
Notifications
You must be signed in to change notification settings - Fork 3
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
Add icon to page header if an account is a contract #348 #350
Conversation
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.
Thanks for the PR! A couple of small changes can be made, but otherwise it looks good.
<button | ||
onclick={goToContract} | ||
class={cn( | ||
'peer absolute left-1/2 top-1/2 -translate-x-1/2 -translate-y-1/2 focus-visible:outline-none', | ||
buttonSize | ||
)} | ||
> | ||
<!-- Button is done this way with absolute positioning so we can maintain a decent hit slop on mobile without affecting layout --> | ||
</button> |
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.
Navigation should be done with a link (anchor tag) when possible to take advantage of preloading
<button | ||
onclick={goToContract} | ||
class={cn( | ||
'peer absolute left-1/2 top-1/2 -translate-x-1/2 -translate-y-1/2 focus-visible:outline-none', | ||
buttonSize | ||
)} | ||
> | ||
<!-- Button is done this way with absolute positioning so we can maintain a decent hit slop on mobile without affecting layout --> | ||
</button> |
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.
Also should be a link
{#if routePath === 'account'} | ||
<CopyButton data={props.title} /> | ||
<CopyButton data={props.title} slop={false} /> | ||
{#if props.contract} | ||
<CodeButton data={contractPath} slop={false} /> | ||
{/if} | ||
{/if} | ||
{#if routePath === 'contract'} | ||
<AccountButton data={accountPath} slop={false} /> | ||
{/if} |
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.
With this layout, the button under the cursor changes depending on the page - we should keep consistency and either add a copy button to the contract page, or swap the order so the switcher button is first.
Merged. Opened issue #372 to track the requested changes. |
Resolve #348