-
-
Notifications
You must be signed in to change notification settings - Fork 8
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
Tournament page layout changes #253
base: main
Are you sure you want to change the base?
Conversation
- Left align title, status, info(image or location + date) - Add icons, skeletons for location and date
|
Also the tournament name is fully missing 😆 |
i just commented it out to show the skeletons lol, i haven't put skeletons for the title yet 😆 |
|
Banner as in where we have tournament title image.. like in beach nationals in Prod |
ah yes. about the image... should we fix these images to a particular size ? this way we can show like a gray background till the tournament query resolves |
hmm so currently we have it width as full and height will be taken as per the aspect ratio(but we can always keep these images as square 1:1 when uploading). I think we can retain that same logic and show a square skeleton with same side length as component width? This will solve all probs like shifting and all |
But this is for mobile.. might have to think diff for desktops. But for now this will be fine xD |
hmm, square skeleton works, but the image might look squished or stretched no ? |
We'll make sure we are always creating and uploading square images only for tournament |
We can tell Comps / Ops also to follow it |
- Also fixed padding for initial and spirit standings
c22f49f
to
6e9078b
Compare
- Left aligned section buttons and added icons - Added skeleton for (almost) whole page
6e9078b
to
4137cd6
Compare
Adding a custom upload widget that crops image might work nicely. I'm not sure if there's something that works well with Solid, but might be worth looking for one. This UX is easily understood because a lot of apps enforce this when uploading profile pictures, etc. |
with tournament banner since we don't know whether a tournament has an image or not, page skeleton wise i'm just showing the image skeleton here. if a tournament doesnt have an image, things might feel like they are shifting upward, as seen in the Surat tournament in the video. Having like an animation between the skeleton and the correct page might alleviate this. ig enough for now ? @Joe2k |
e80188c
to
ee765ec
Compare
we could just put a max-width on the image ? |
ee765ec
to
ba0764d
Compare
Also have set a max-width on the images
ba0764d
to
e6e60a9
Compare
@@ -144,147 +185,153 @@ const Tournament = () => { | |||
icon={trophy} | |||
pageList={[{ url: "/tournaments", name: "All Tournaments" }]} | |||
/> | |||
<Suspense fallback={<TournamentPageSkeleton />}> |
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 class="flex justify-start"> |
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.
Can we have the logo still centered. Again since its taking full width anyway it should be fine. Otherwise feels its out of center by few inches
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.
The image is taking 3/4 of container width no ?
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.
ahh cool.. Then lets make it full width of the container? That should be good i think
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.
hmm i still like full width more.. yes for larger screens we have to think diff. But what i meant before was image left aligned is not looking too good since its a square. Would be perfect if it was rectangle and didnt have any space left on the right. But since its square, not centering it looks off. Like I havent seen image left aligned in any other websites with space left on the right
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.
correct, the square image looks a bit off when left centered. I was talking about left aligning for larger screens only. Agreed.
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.
hmm i still like full width more.. yes for larger screens we have to think diff. But what i meant before was image left aligned is not looking too good since its a square. Would be perfect if it was rectangle and didnt have any space left on the right. But since its square, not centering it looks off. Like I havent seen image left aligned in any other websites with space left on the right
hmm, maybe we should enforce a 4/3 or 16/9 aspect ratio then ?
@@ -353,11 +399,11 @@ const Tournament = () => { | |||
<tr class="border-b bg-white dark:border-gray-700 dark:bg-gray-800"> | |||
<th | |||
scope="row" | |||
class="whitespace-nowrap py-4 pl-10 pr-6 font-normal" | |||
class="whitespace-nowrap py-4 pl-4 pr-2 font-normal" |
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.
Screen.Recording.2024-01-18.at.7.26.56.PM.mov
There is a bit of mismatch in padding between final and initial
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 pl here can be 6 / 8..Rank feels too close to the left
|
||
<div class="mb-4 border-b border-gray-200 dark:border-gray-700"> | ||
<div class="mb-4 border-b border-gray-200 px-1 dark:border-gray-700"> |
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.
I feel lets park this for now... There is too much bias in lot of things i feel cause we ourselves dont know the best practices to design the UI. Lets properly do a design doc first in figma for all kinds of devices and then pick this? Cause anyway the other pages like schedule and standings and rules are all still centered so might not look good when transitioning @SibiAkkash what you think? |
yeah makes sense, the other pages would also have to be changed. Alright, lets have a plan and do this at once. |
tournament-page-2024-01-16_22.59.12.mp4
Changes as discussed here: https://upai.zulipchat.com/#narrow/stream/387757-tech-backend/topic/tournament.20page.20layout
Bug: Wrapping the full page in a
Suspense
breaks the flowbite components from loading.When we wrap the whole thing in Suspense, the first few setTimeouts in onMount don't have any effect. Adding only a setTimeout of say 8 seconds highlights this bug. Only after 8 seconds, do the flowbite tabs actually render. 😬
The setTimout(s) in
onMount
is a bit of hack and it works because the tournament query latency is ~ equal to setTimeout delays.@Joe2k @punchagan should probably look for different library from flowbite later on....