-
Notifications
You must be signed in to change notification settings - Fork 6
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
Feature/our sponsors - made layout to match figma design #165
Conversation
I think you accidentally posted the wrong mobile view screenshot (this one doesn't show the component lol) |
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.
Left comments related to spacing. Pretty small though. Nice job! 💯
<h1 className="lg:text-5xl text-6xl mb-12"> | ||
<strong>Sponsors and Partners</strong> | ||
<section | ||
className="bg-negative text-negative flex flex-col justify-evenly py-16 max-lg:h-fit max-lg:gap-y-6 max-lg:py-16" |
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 think we could add some horizontal padding to this section (to have it align with the padding of the right section of the hero page so using px-20
). It just helps with making the page look more professional and presentable to these NPOs as this home page could be their first time interacting with our group yknow.
className="bg-negative text-negative flex flex-col justify-evenly py-16 max-lg:h-fit max-lg:gap-y-6 max-lg:py-16" | ||
id="sponsors" | ||
> | ||
<h1 className="lg:text-5xl text-6xl mt-10 mb-2 ml-5 text-black text-left"> |
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.
Similar comment to the comments made in the #163. Text should be larger on mobile view. If you addressed the above comment, keep in mind that the added padding will need to be adjusted to be smaller in mobile view too.
Some comments were unresolved and these changes were addressed in another more recent PR #199 |
Closes #164
Reference
Figma Mockup
Desktop View
Mobile View