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

Tournament page layout changes #253

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

SibiAkkash
Copy link
Contributor

@SibiAkkash SibiAkkash commented Jan 15, 2024

  • Reduced padding for standings table
  • Left aligned tournament info section
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....

- Left align title, status, info(image or location + date)
- Add icons, skeletons for location and date
@SibiAkkash SibiAkkash requested a review from Joe2k January 15, 2024 07:01
@SibiAkkash SibiAkkash self-assigned this Jan 15, 2024
@Joe2k
Copy link
Contributor

Joe2k commented Jan 15, 2024

  1. Can you attach a ss with the banner in tournament page as well?
  2. Should we left align the tabs as well? Currently looks weird as tabs are centered in the page while everything else is left aligned

@Joe2k
Copy link
Contributor

Joe2k commented Jan 15, 2024

Also the tournament name is fully missing 😆

@SibiAkkash
Copy link
Contributor Author

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 😆

@SibiAkkash
Copy link
Contributor Author

  1. Can you attach a ss with the banner in tournament page as well?
  2. Should we left align the tabs as well? Currently looks weird as tabs are centered in the page while everything else is left aligned
  1. banner ?
  2. yes, trying left aligned tabs now

@Joe2k
Copy link
Contributor

Joe2k commented Jan 15, 2024

Banner as in where we have tournament title image.. like in beach nationals in Prod

@SibiAkkash
Copy link
Contributor Author

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

@Joe2k
Copy link
Contributor

Joe2k commented Jan 15, 2024

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

@Joe2k
Copy link
Contributor

Joe2k commented Jan 15, 2024

But this is for mobile.. might have to think diff for desktops. But for now this will be fine xD

@SibiAkkash
Copy link
Contributor Author

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

hmm, square skeleton works, but the image might look squished or stretched no ?

@Joe2k
Copy link
Contributor

Joe2k commented Jan 15, 2024

but the image might look squished or stretched no ?

We'll make sure we are always creating and uploading square images only for tournament

@Joe2k
Copy link
Contributor

Joe2k commented Jan 15, 2024

We can tell Comps / Ops also to follow it

- Also fixed padding for initial and spirit standings
@SibiAkkash SibiAkkash force-pushed the standings-comp branch 2 times, most recently from c22f49f to 6e9078b Compare January 16, 2024 16:42
- Left aligned section buttons and added icons
- Added skeleton for (almost) whole page
@punchagan
Copy link
Member

We can tell Comps / Ops also to follow it

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.

@SibiAkkash
Copy link
Contributor Author

  1. Can you attach a ss with the banner in tournament page as well?
  2. Should we left align the tabs as well? Currently looks weird as tabs are centered in the page while everything else is left aligned

with tournament banner
https://github.com/india-ultimate/hub/assets/39093537/52ef5c69-9144-4273-b243-6c2f84d56e89

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

@SibiAkkash SibiAkkash force-pushed the standings-comp branch 2 times, most recently from e80188c to ee765ec Compare January 17, 2024 13:51
@SibiAkkash
Copy link
Contributor Author

But this is for mobile.. might have to think diff for desktops. But for now this will be fine xD

we could just put a max-width on the image ?

Also have set a max-width on the images
@@ -144,147 +185,153 @@ const Tournament = () => {
icon={trophy}
pageList={[{ url: "/tournaments", name: "All Tournaments" }]}
/>
<Suspense fallback={<TournamentPageSkeleton />}>
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we have the tournament status in text-xs and under the title. Feel like its dominating a lot of space otherwise
Screenshot 2024-01-18 at 7 22 57 PM

/>
}
>
<div class="flex justify-start">
Copy link
Contributor

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

Copy link
Contributor Author

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 ?

Copy link
Contributor

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

if we center it, it would look off on larger screens right ? everything else if left aligned regardless of screensize. Also, the image being off center might be because the beach nats image has a transparent border ?

Beach nats Surat
image image
left-align center-align
image image

Copy link
Contributor

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

Copy link
Contributor Author

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.

Copy link
Contributor Author

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"
Copy link
Contributor

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

Copy link
Contributor

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">
Copy link
Contributor

Choose a reason for hiding this comment

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

Screenshot 2024-01-18 at 7 28 29 PM

The underline below the tabs are not at the centre of words but more aligned to the right

@Joe2k
Copy link
Contributor

Joe2k commented Jan 19, 2024

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?

@SibiAkkash
Copy link
Contributor Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants