Tournament Discovery #3955
Tournament Discovery #3955
Conversation
ncarazon
force-pushed
the
feat/tournaments-discovery
branch
from
66ca630 to
eaf749b
Compare
ncarazon
had a problem deploying
to
testing_env
GitHub Actions
Failure
ncarazon
force-pushed
the
feat/tournaments-discovery
branch
from
eaf749b to
d2bf86e
Compare
ncarazon
had a problem deploying
to
testing_env
GitHub Actions
Failure
ncarazon
force-pushed
the
feat/tournaments-discovery
branch
from
d2bf86e to
f6837fd
Compare
|
|
||
| return ( | ||
| <TournamentCardShell item={item}> | ||
| <div className="relative h-[64px] w-full bg-blue-100/40 dark:bg-blue-100-dark/20 lg:h-[80px]"> |
There was a problem hiding this comment.
Any reason for not using the tw classes for h-16 and h-20?
There was a problem hiding this comment.
same comment in the other remaining places
| const LiveTournamentCard: React.FC<Props> = ({ item, nowTs = 0 }) => { | ||
| const t = useTranslations(); | ||
| const prize = useMemo( | ||
| () => formatMoneyUSD(item.prize_pool ?? null), |
There was a problem hiding this comment.
minor, but formatMoneyUSD seems to accept both null an undefined
|
|
||
| if (nowTs == null) { | ||
| label = t("tournamentTimelineOngoing"); | ||
| p = 0; |
There was a problem hiding this comment.
redundant p=0 here and below
| const pick = () => { | ||
| if (deltaMs < hour) | ||
| return { n: Math.round(deltaMs / min), unit: "minute" as const }; | ||
| if (deltaMs < day) | ||
| return { n: Math.round(deltaMs / hour), unit: "hour" as const }; | ||
| if (deltaMs < week) | ||
| return { n: Math.round(deltaMs / day), unit: "day" as const }; | ||
| if (deltaMs < month) | ||
| return { n: Math.round(deltaMs / week), unit: "week" as const }; | ||
| if (deltaMs < year) | ||
| return { n: Math.round(deltaMs / month), unit: "month" as const }; | ||
| return { n: Math.round(deltaMs / year), unit: "year" as const }; | ||
| }; |
There was a problem hiding this comment.
We have truncateDuration in front_end/src/utils/formatters/date.ts - I haven't looked very closely, but maybe it can be used here?
| return ( | ||
| <div | ||
| className={cn( | ||
| ` |
There was a problem hiding this comment.
Interesting format you chose here - any reason why? Just curious, since it's different from the other places in our code
| value={draftQuery} | ||
| onChange={handleSearchChange} | ||
| onErase={handleSearchErase} | ||
| placeholder="search..." |
| const [draftQuery, setDraftQuery] = useState(searchQuery); | ||
| useEffect(() => setDraftQuery(searchQuery), [searchQuery]); |
There was a problem hiding this comment.
It's not clear why the extra draftQuery state/setState is needed here. If that's really needed, can you leave a small comment to clarify? Although, at first sight, it looks like it's not needed.
|
|
||
| const getInputEl = useCallback( | ||
| () => | ||
| rootRef.current?.querySelector<HTMLInputElement>('input[type="search"]'), |
There was a problem hiding this comment.
Any reason not to keep a ref directly to the search input field instead of doing this?
| onChange={onChange} | ||
| onErase={() => { | ||
| onErase(); | ||
| if (keepOpenWhenHasValue) setOpen(false); |
There was a problem hiding this comment.
At first sight (read of the variable names), this seems wrong, but probably it's correct, so ignore this comment if you're sure it's the correct behaviour.
| @@ -0,0 +1,19 @@ | |||
| import { useEffect, useState } from "react"; | |||
|
|
|||
| export function useIsInViewport(el: HTMLElement | null) { | |||
There was a problem hiding this comment.
This hook doesn't seem to be used anywhere?
Closes #3952
This PR redesigns tournaments page
search_demo.mov