Updated Sponsor Styling #154
Conversation
|
Deploy preview for zothacks-site-2023-sanity ready!
|
|
Deploy preview for zothacks-site-2023 ready!
|
| @include utils.media-breakpoint-down(sm) { | ||
| font-size: 2rem; | ||
| } |
There was a problem hiding this comment.
Suggestion [repeating]: Bootstrap recommends a "mobile-first" design approach, so making the narrower view the default and using media-breakpoint-up would be more appropriate.
| flex-wrap: wrap; | ||
| justify-content: center; | ||
| gap: 1.5rem; | ||
| max-width: 1200px; |
There was a problem hiding this comment.
Suggestion: rather than manually constraining the inner elements, we can use Bootstrap's native container sizing (unfortunately not done last year) by changing the main <section> element to be a Container, see Home Intro (Intro.tsx:14) for example.
| width: 100%; | ||
| height: 100%; | ||
| display: flex; | ||
| align-items: center; | ||
| justify-content: center; |
There was a problem hiding this comment.
Suggestion: since the parent elements have .logo class applied, the centered placement should already be taken care of, so these shouldn't need to be specified.
There was a problem hiding this comment.
Think you also have to remove the forced dimensions (width and height) for it to be centered by parent
…implementation with project's mobile-first design approach
| // .container { | ||
| // padding-top: 6rem; | ||
| // padding-bottom: 6rem; | ||
| // } |
There was a problem hiding this comment.
Chore: little value in keeping around old commented code, better to just remove it
| width: 100%; | ||
| height: 100%; | ||
| display: flex; | ||
| align-items: center; | ||
| justify-content: center; |
There was a problem hiding this comment.
Think you also have to remove the forced dimensions (width and height) for it to be centered by parent
| @@ -3,6 +3,7 @@ import { getSponsors } from "./getSponsors"; | |||
| import styles from "./Sponsors.module.scss"; | |||
| import { client } from "@/lib/sanity/client"; | |||
| import imageUrlBuilder from "@sanity/image-url"; | |||
| import { Container } from "react-bootstrap"; | |||
There was a problem hiding this comment.
Chore: I forget if Next.js handles this well, but to be safe, we'll want to import just the individual component, i.e. import Container from "react-bootstrap/Container"
Import individual components rather than the entire library. Doing so pulls in only the specific components that you use (React Bootstrap, Importing Components)
Description
Used existing CMS functions for new sponsor section. Updated styling to make it responsive. Added hover effect for fun!!!!
Screenshots
Steps to verify/test this change:
Final Checks:
Issues
Closes #146
Todo
Nice to have
Maybe add more animations??