-
Notifications
You must be signed in to change notification settings - Fork 17
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
Prettify action details in several action views #767
Conversation
</div> | ||
|
||
<div className='max-w-72 truncate'> | ||
<AddressComponent address={a.address} /> |
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.
<AddressComponent />
no longer shortens addresses to a specific number of characters — instead, it's more flexible, expanding or contracting to fit its container. So we'll manually narrow it via a parent <div />
here.
@@ -7,7 +7,7 @@ export interface CardProps extends React.HTMLAttributes<HTMLDivElement> { | |||
|
|||
const Card = React.forwardRef<HTMLDivElement, CardProps>( | |||
({ className, gradient, children, ...props }, ref) => { | |||
const baseClasses = 'bg-charcoal rounded-lg shadow-sm p-[30px]'; | |||
const baseClasses = 'bg-charcoal rounded-lg shadow-sm p-[30px] 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.
To ensure truncating strings works
export const ActionDetails = ({ children, label }: { children: ReactNode; label?: string }) => { | ||
return ( | ||
<div className='flex flex-col gap-2'> | ||
{!!label && <div className='font-bold'>{label}</div>} |
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.
<div> | ||
<span className='font-bold'>Epoch index:</span> {value.epochIndex.toString()} | ||
</div> | ||
<ActionDetails> |
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.
<b>Asset 1:</b> | ||
<div className='ml-5'> | ||
<b>ID: </b> | ||
<div className='flex flex-col gap-8'> |
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.
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.
(In follow-up PR(s), this will move closer to this proposed design.)
<div> | ||
<span className='font-bold'>Epoch index:</span> {value.startEpochIndex.toString()} | ||
</div> | ||
<ActionDetails> |
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.
@@ -9,6 +9,8 @@ export interface ViewBoxProps { | |||
visibleContent?: React.ReactElement; | |||
} | |||
|
|||
const Label = ({ label }: { label: string }) => <span className='text-lg'>{label}</span>; |
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.
Just making the label a little bigger, to differentiate from the view box's contents
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.
Nice job! One comment.
Didn't see your comment @grod220 😆 let's address in a follow-up PR? |
Oh shoot, I must have not clicked "save" on that comment. It was more of a thought that the tx details page should likely not truncate without giving a way for the user to view the entire value. For instance, we should either have a design that supports arbitrary characters, or a tooltip that shows the full value on hover. |
A lot of our action details look a bit ugly. I created
<ActionDetails />
in a previous PR to address this, but I'd only used it for a single action type. This PR uses it in more places.This doesn't resolve all our issues with how we display swaps + swap claims (more detail in #424), but it's a start.
Comments to explain my choices are inline.
Relates to #424