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

Move club cards display to masonry layout #711

Merged
merged 3 commits into from
Oct 13, 2024

Conversation

aviupadhyayula
Copy link
Member

The front page currently displays club cards in two columns, with each "row" of cards beginning at the same point. This creates gaps when one column contains cards with longer lengths than the other. This PR moves to a masonry layout.

Here's a before vs after comparison. Before:
Screenshot 2024-08-23 at 6 32 27 PM
After:
Screenshot 2024-08-23 at 6 32 34 PM

<StyledMasonry
breakpointCols={{
default: 2,
960: 1,
Copy link
Member

@julianweng julianweng Aug 24, 2024

Choose a reason for hiding this comment

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

Ideally use some existing constant to define sm screen sizes

default: 2,
960: 1,
}}
className="masonry-grid"
Copy link
Member

Choose a reason for hiding this comment

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

Is the classname for the parent Masonry component ever used?

Copy link
Member

@julianweng julianweng left a comment

Choose a reason for hiding this comment

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

Thanks Avi! Using a pure CSS library like this that doesn't re-order to optimize for height is probably the right move from a performance standpoint as well as maintaining rankings. Made some nits.

@julianweng julianweng merged commit b4ea850 into master Oct 13, 2024
3 checks passed
@julianweng julianweng deleted the avi/add-masonry-to-front-page branch October 13, 2024 03:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

2 participants