STAT-931 - Create reusable hero component for all pages #46
Conversation
src/components/Hero.astro
Outdated
| --- | ||
| <div class={containerClasses}> | ||
| <div class="w-12/12 lg:order-last lg:w-6/12"> | ||
| <Image src={heroImage} alt="hero image" densities={[1, 2, 3, 4]} /> |
There was a problem hiding this comment.
@Mat-Mwi - let's change the alt text from "hero image" to the page headline.
src/components/Hero.astro
Outdated
| const containerClasses = `container flex flex-wrap mx-auto px-[0.5rem] sm:px-[1rem] md:px-[1.5rem] lg:px-[2rem] xl:px-[1.5rem] 2xl:px-[3.62rem] 3xl:px-[6.62rem] ${bottomMargin}`; | ||
| --- | ||
| <div class={containerClasses}> | ||
| <div class="w-12/12 lg:order-last lg:w-6/12"> |
There was a problem hiding this comment.
@Mat-Mwi - if you take a look at this at intermediate sizes, the image becomes so large it pushes the text below the fold. Can you please revisit, creating a div to wrap around the text, centered, and shrank per breakpoint so the image and the text is above the fold at small and medium breakpoints?
There was a problem hiding this comment.
Sorry, I posted #46 (review) before seeing this. Same feedback. 👍
There was a problem hiding this comment.
I have addressed this feedback and it is ready for another look. The images width is restricted on smaller breakpoints and it is centered.
There was a problem hiding this comment.
I'm not sure this fulfills the design intention. We're heading into a 4-day weekend here, let's pick up the conversation next week.
There was a problem hiding this comment.
@chadwhitacre - what screen size is this (~992ish - 1100ish)? I feel like it could fit the desktop design.
There was a problem hiding this comment.
This is at 1023 pixels, one short of the breakpoint.
There was a problem hiding this comment.
@chadwhitacre Great, we're going to switch to desktop view as Brian suggested and see if that works better. I'm going to see if I can have this ready for review by Tuesday.
src/components/Hero.astro
Outdated
| <Image class="w-[30rem] mx-auto md:w-full" src={heroImage} alt={headline} densities={[1, 2, 3, 4]} /> | ||
| </div> | ||
| <div class="w-12/12 mb-[1.5rem] mt-[0.78375rem] md:my-auto md:pr-[2.02625rem] md:w-1/2"> | ||
| <h1 class="mb-[1rem] text-[2.5rem] text-black 2xl:text-[3.75rem]">{headline}</h1> |
There was a problem hiding this comment.
@Mat-Mwi can you please adjust the font sizes for breakpoint md as noted here: https://www.figma.com/design/q8NAB0WClWHGTbajxKeskD/Sentry---Fair.io-(8)?node-id=1015-1377&node-type=frame&m=dev
No description provided.