-
Notifications
You must be signed in to change notification settings - Fork 0
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
feat: UI for all screens #2
base: master
Are you sure you want to change the base?
Conversation
<Button className="bg-hathor-purple-500 w-full text-white max-w-md mt-12 h-12 text-md"> | ||
Set result | ||
</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.
question: Shouldn't it be conditional to user's wallet address connected?
|
||
return ( | ||
<main className="flex min-h-screen items-center p-6 flex-col"> | ||
<Header logo={false} title='Betting' subtitle="Olympic Games - Men's Football Finals" /> |
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.
question: Shouldn't the "Olympic Games - Men's Football Finals" text be dynamic? Isn't it the Nano Contract name?
defaultValues: { | ||
name: '', | ||
description: '', | ||
oracleType: 'random', |
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.
question: What an oracleType
'random' means?
<div className='bg-black '> | ||
</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.
question: Where is the image? Shouldn't it be among the project's assests?
<Button className='bg-hathor-purple-500 w-full text-white text-md h-12'>Collect your prize</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.
question: Shouldn't it be conditional to the winner address?
return ( | ||
<div className='flex justify-between w-full max-w-md'> | ||
<p className='text-md subpixel-antialiased'>Total Bet</p> | ||
<p className='text-md subpixel-antialiased'>200 EVC</p> |
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.
question: Is it fixed?
cell: ({ row }) => RowCapitalizeFirst(formatDate(row.getValue('timestamp'))), | ||
}]; |
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.
polish: Timestamp doesn't need capitalize, maybe you can use the value directly.
const SortableTableHeader = (name: string, column: Column<TransactionHistoryItem, unknown>) => { | ||
const Arrow = (sorted: false | string) => { | ||
console.log('sorted', sorted); | ||
switch(sorted) { | ||
case 'asc': | ||
return <ArrowUp className="ml-2 h-4 w-4 text-hathor-purple-500" /> | ||
case 'desc': | ||
return <ArrowDown className="ml-2 h-4 w-4 text-hathor-purple-500" /> | ||
default: | ||
return <ArrowUpDown className="ml-2 h-4 w-4 text-hathor-purple-500" /> | ||
} | ||
}; |
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.
suggestion: Remove as it is not used.
I'm setting it as a suggestion because I don't really think it is the case to block the PR because of it.
<TableHead key={header.id}> | ||
{header.isPlaceholder | ||
? null | ||
: flexRender( |
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.
question: What is a flexRender
?
const connected = false; | ||
const [connected, setConnected] = React.useState<boolean>(false); | ||
const onConnect = (e: React.MouseEvent<HTMLButtonElement>) => { | ||
e.preventDefault(); |
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.
question: Why should we prevent default behavior here?
Acceptance Criteria