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

views: improve approval dialogue rendering #241

Merged
merged 8 commits into from
Nov 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 17 additions & 4 deletions apps/extension/src/routes/popup/approval/approve-deny.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,25 @@ export const ApproveDeny = ({
const count = useWindowCountdown(wait);

return (
<div className='flex flex-row flex-wrap justify-center gap-4 bg-black p-4 shadow-lg'>
<Button variant='gradient' className='w-full' size='lg' onClick={approve} disabled={!!count}>
<div
className='flex flex-row justify-between gap-4 rounded-md p-4 shadow-md'
style={{
backgroundColor: '#1A1A1A',
paddingBottom: '28px',
paddingTop: '28px',
}}
Comment on lines +19 to +24
Copy link
Contributor Author

Choose a reason for hiding this comment

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

blending the colors of the layer containing the approve and deny buttons


before

Screenshot 2024-11-24 at 9 45 58 PM Screenshot 2024-11-24 at 9 47 19 PM

after

Screenshot 2024-11-24 at 9 42 50 PM Screenshot 2024-11-24 at 9 42 38 PM

TalDerei marked this conversation as resolved.
Show resolved Hide resolved
>
<Button
variant='gradient'
className='w-1/2 py-3.5 text-base'
size='lg'
onClick={approve}
disabled={!!count}
>
Approve {count !== 0 && `(${count})`}
</Button>
<Button
className='min-w-[50%] grow items-center gap-2 hover:bg-destructive/90'
className='w-1/2 py-3.5 text-base hover:bg-destructive/90'
size='lg'
variant='destructiveSecondary'
onClick={deny}
Expand All @@ -29,7 +42,7 @@ export const ApproveDeny = ({
</Button>
{ignore && (
<Button
className='w-1/3 hover:bg-destructive/90'
className='w-1/2 py-3.5 text-base hover:bg-destructive/90'
size='lg'
variant='secondary'
onClick={ignore}
Expand Down
17 changes: 10 additions & 7 deletions apps/extension/src/routes/popup/approval/transaction/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -47,11 +47,12 @@ export const TransactionApproval = () => {

return (
<div className='flex h-screen flex-col'>
<div className='grow overflow-auto p-[30px] pt-10'>
<p className='bg-text-linear bg-clip-text pb-2 font-headline text-2xl font-bold text-transparent'>
Confirm transaction
</p>

<div className='border-b border-gray-700 p-4'>
<h1 className=' bg-text-linear bg-clip-text pb-0 font-headline text-2xl font-bold text-transparent'>
Confirm Transaction
</h1>
</div>
<div className='grow overflow-auto p-4'>
<ViewTabs
defaultValue={selectedTransactionViewName}
onValueChange={setSelectedTransactionViewName}
Expand All @@ -60,13 +61,15 @@ export const TransactionApproval = () => {
<TransactionViewComponent txv={selectedTransactionView} metadataFetcher={getMetadata} />

{selectedTransactionViewName === TransactionViewTab.SENDER && (
<div className='mt-8'>
<div className='mt-4'>
<JsonViewer jsonObj={authorizeRequest.toJson() as Jsonified<AuthorizeRequest>} />
</div>
)}
</div>

<ApproveDeny approve={approve} deny={deny} wait={3} />
<div className='border-t border-gray-700 p-0'>
<ApproveDeny approve={approve} deny={deny} wait={3} />
</div>
</div>
);
};
6 changes: 3 additions & 3 deletions apps/extension/src/routes/popup/home/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -50,16 +50,16 @@ export const PopupIndex = () => {
<>
<BlockSync />

<div className='flex h-full grow flex-col items-stretch gap-[30px] bg-logo bg-left-bottom px-[30px] pb-[30px]'>
<div className='flex h-full grow flex-col items-stretch gap-[15px] bg-logo bg-left-bottom px-[15px] pb-[15px]'>
<IndexHeader />

<div className='flex flex-col gap-8'>
<div className='flex flex-col gap-4'>
{activeWallet && <SelectAccount getAddrByIndex={getAddrByIndex(activeWallet)} />}
</div>

<ValidateAddress />

<div className='grow' />
<div className='shrink-0 grow' />

<FrontendLink />
</div>
Expand Down
2 changes: 1 addition & 1 deletion packages/ui/components/ui/tabs/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ const TabsList = React.forwardRef<
<TabsPrimitive.List
ref={ref}
className={cn(
'inline-flex h-[52px] items-center justify-center rounded-lg bg-background px-2',
'inline-flex h-[52px] items-center justify-center rounded-lg bg-background px-2 mb-4',
className,
)}
{...props}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ const ActionDetailsRow = ({
<span>{label}</span>
</span>
) : (
<span className='whitespace-nowrap break-keep'>{label}</span>
<span className='whitespace-nowrap break-keep text-base'>{label}</span>
)}

<Separator />
Expand Down
7 changes: 5 additions & 2 deletions packages/ui/components/ui/tx/actions-views/output.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,12 @@ export const OutputViewComponent = ({ value }: { value: OutputView }) => {
<ViewBox
label='Output'
visibleContent={
<ValueWithAddress addressView={address} label='to'>
<div className='flex items-center justify-between gap-3'>
<ValueViewComponent view={note.value} />
</ValueWithAddress>
<ValueWithAddress addressView={address} label='to'>
<></>
</ValueWithAddress>
</div>
Comment on lines +17 to +22
Copy link
Contributor

Choose a reason for hiding this comment

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

issue: a <></> is basically empty. Also, think addresses are getting cut off when there should be a line break likely.

Screenshot 2024-11-25 at 10 58 44 AM

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch! This was refactored to place everything on the same line, but it unintentionally cuts off transfers to external wallet addresses. I truncated the ValueWithAddress component with a max width to fix this.

Screenshot 2024-11-25 at 8 32 43 AM Screenshot 2024-11-25 at 8 32 58 AM

}
/>
);
Expand Down
9 changes: 6 additions & 3 deletions packages/ui/components/ui/tx/actions-views/spend.tsx
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import { SpendView } from '@penumbra-zone/protobuf/penumbra/core/component/shielded_pool/v1/shielded_pool_pb';
import { ViewBox } from '../viewbox';
import { ValueViewComponent } from '../../value';
import { ValueWithAddress } from './value-with-address';
import { getNote } from '@penumbra-zone/getters/spend-view';
import { getAddress } from '@penumbra-zone/getters/note-view';
import { ValueViewComponent } from '../../value';

export const SpendViewComponent = ({ value }: { value: SpendView }) => {
if (value.spendView.case === 'visible') {
Expand All @@ -14,9 +14,12 @@ export const SpendViewComponent = ({ value }: { value: SpendView }) => {
<ViewBox
label='Spend'
visibleContent={
<ValueWithAddress addressView={address} label='from'>
<div className='flex items-center justify-between gap-3'>
<ValueViewComponent view={note.value} />
</ValueWithAddress>
<ValueWithAddress addressView={address} label='from'>
<></>
</ValueWithAddress>
Comment on lines +19 to +21
Copy link
Contributor

Choose a reason for hiding this comment

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

question: This is the same as

  <ValueWithAddress addressView={address} label='from' />

But is that intended?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I changed it since It was throwing an error related to missing attributes, but functionally it's the same.

Screenshot 2024-11-25 at 8 36 51 AM

</div>
}
/>
);
Expand Down
2 changes: 1 addition & 1 deletion packages/ui/components/ui/tx/actions-views/swap/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ export const SwapViewComponent = ({
)}

<ActionDetails.Row label='Swap Claim Fee'>
<div className='font-mono'>
<div className='text-base'>
<ValueViewComponent view={prepaidClaimFee} />
</div>
</ActionDetails.Row>
Expand Down
14 changes: 8 additions & 6 deletions packages/ui/components/ui/tx/actions-views/swap/one-way-swap.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import { ValueViewComponent } from '../../../value';
import { ArrowRight } from 'lucide-react';
import { ValueView } from '@penumbra-zone/protobuf/penumbra/core/asset/v1/asset_pb';
import { getAmount } from '@penumbra-zone/getters/value-view';

Expand All @@ -13,12 +12,15 @@ export const OneWaySwap = ({ input, output }: { input: ValueView; output: ValueV
const outputAmount = getAmount.optional(output);

return (
<div className='flex items-center gap-2'>
<div className='flex items-center justify-between'>
<ValueViewComponent view={input} />

<ArrowRight />

<ValueViewComponent view={output} showValue={!!outputAmount} />
<div className='relative mx-2 flex items-center justify-center'>
<div className='flex h-0.5 w-3.5 items-center bg-white' />
<div className='ml-[2px] size-0 border-y-4 border-l-[6px] border-y-transparent border-l-white' />
</div>
<div className='flex items-center justify-end'>
<ValueViewComponent view={output} showValue={!!outputAmount} />
</div>
Copy link
Contributor

Choose a reason for hiding this comment

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

issue: swap-claim fee seems to have a different font

Screenshot 2024-11-25 at 10 53 48 AM

Copy link
Contributor Author

Choose a reason for hiding this comment

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

modified to text-base – suggesting we conduct an audit of the typography styles being used and and cross-reference with the figma designs per penumbra-zone/web#1921.

</div>
);
};
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ export const ValueWithAddress = ({
label,
addressView,
}: {
/** What to display before the address. Typically a `ValueViewComponent`. */
children: ReactNode;
label: 'from' | 'to';
addressView?: AddressView;
Expand All @@ -22,8 +21,9 @@ export const ValueWithAddress = ({
{addressView && (
<div className='flex items-center gap-2 overflow-hidden'>
<span className='whitespace-nowrap font-mono text-sm italic text-foreground'>{label}</span>

<AddressViewComponent view={addressView} />
<div className='max-w-[150px] truncate'>
<AddressViewComponent view={addressView} />
</div>
</div>
)}
</div>
Expand Down
6 changes: 3 additions & 3 deletions packages/ui/components/ui/tx/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -64,14 +64,14 @@ export const TransactionViewComponent = ({
const { feeValueView, isLoading, error } = useFeeMetadata(txv, metadataFetcher);

return (
<div className='flex flex-col gap-8'>
<div className='flex flex-col gap-4'>
{txv.bodyView?.memoView?.memoView && <MemoViewComponent memo={txv.bodyView.memoView} />}
<ViewSection heading='Actions'>
<ViewSection heading={<div className='pl-[3px]'>Actions</div>}>
{txv.bodyView?.actionViews.map((av, i) => (
<ActionViewComponent av={av} feeValueView={feeValueView} key={i} />
))}
</ViewSection>
<ViewSection heading='Parameters'>
<ViewSection heading={<div className='pl-[3px]'>Parameters</div>}>
<ViewBox
label='Transaction Fee'
visibleContent={
Expand Down
4 changes: 2 additions & 2 deletions packages/ui/components/ui/tx/memo-view.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,13 @@ export const MemoViewComponent = ({ memo: { memoView } }: { memo: MemoView }) =>
<ViewBox
label='Memo'
visibleContent={
<div className='flex flex-col gap-4'>
<div className='flex flex-col gap-2'>
<ActionDetails>
<ActionDetails.Row label='Return Address'>
<AddressViewComponent view={memoView.value.plaintext!.returnAddress} />
</ActionDetails.Row>
<ActionDetails.Row label='Memo Text'>
<span className='italic' style={{ wordBreak: 'normal' }}>
<span className='text-sm italic' style={{ wordBreak: 'normal' }}>
{memoView.value.plaintext?.text}
</span>
</ActionDetails.Row>
Expand Down
31 changes: 15 additions & 16 deletions packages/ui/components/ui/tx/viewbox.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,44 +11,43 @@ export interface ViewBoxProps {
isOpaque?: boolean;
}

const Label = ({ label }: { label: string }) => <span className='text-lg'>{label}</span>;
const Label = ({ label }: { label: string }) => (
<span className='text-lg font-bold text-gray-300'>{label}</span>
);

export const ViewBox = ({ label, visibleContent, isOpaque }: ViewBoxProps) => {
// if isOpaque is undefined, set it to !visibleContent
isOpaque = isOpaque ?? !visibleContent;
isOpaque = isOpaque ?? !visibleContent?.props;

return (
<Box overflow='hidden'>
<div
className={cn(
'flex flex-col gap-1 break-all overflow-hidden',
isOpaque ? 'cursor-not-allowed' : '',
)}
>
<div className='flex items-center gap-2 self-start'>
<span className={cn('text-base font-bold', isOpaque ? 'text-gray-600' : '')}>
{!isOpaque && <Label label={label} />}
{isOpaque && (
<div className='flex gap-2'>
<div className={cn('flex flex-col gap-2', isOpaque ? 'cursor-not-allowed' : '')}>
<div className='flex items-center gap-2'>
<span className={cn('text-base', isOpaque ? 'text-gray-600' : '')}>
{isOpaque ? (
<div className='flex items-center gap-2'>
<IncognitoIcon fill='#4b5563' />
<Label label={label} />
</div>
) : (
<Label label={label} />
)}
</span>
</div>
{visibleContent}
{visibleContent && <div className='border-t border-gray-700 pt-2'>{visibleContent}</div>}
Copy link
Contributor

Choose a reason for hiding this comment

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

suggestion: personally, I think you can get rid of the horizontal line border

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I actually prefer the horizontal line border because it provides clearer visual separation and enhances the structure.

Screenshot 2024-11-25 at 1 33 02 PM Screenshot 2024-11-25 at 1 32 24 PM

</div>
</Box>
);
};

export interface ViewSectionProps {
heading: string;
heading: React.ReactNode;
children?: React.ReactNode;
}

export const ViewSection = ({ heading, children }: ViewSectionProps) => {
return (
<div className='grid gap-4'>
<div className='grid gap-2'>
<div className='text-xl font-bold'>{heading}</div>
{children}
</div>
Expand Down
4 changes: 3 additions & 1 deletion packages/ui/components/ui/value/value.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,9 @@ export const ValueComponent = ({
</span>
)}
{showDenom && (
<span className='truncate font-mono text-xs text-muted-foreground'>{symbol}</span>
<span className='max-w-[80px] truncate font-mono text-xs text-muted-foreground'>
{symbol}
</span>
)}
</div>
</Pill>
Expand Down