Responsive landing #31
Conversation
Codecov Report
@@ Coverage Diff @@
## main #31 +/- ##
=======================================
Coverage 52.19% 52.19%
=======================================
Files 43 43
Lines 3257 3257
=======================================
Hits 1700 1700
Misses 1367 1367
Partials 190 190
Flags with carried forward coverage won't be shown. Click here to find out more. Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
There was a problem hiding this comment.
The responsive behavior of the site is much improved, very nice work!
I did notice a couple of things during my review that I thought I'd point out to see if we could fix or create stories for if necessary.
I am glad we're going to a single column when we move down to xs or sm screens. However, we probably should make the font smaller and still give a bit of margin on the side. Does tailwind have the feature functionality to do this?
Also when you collapse the screen then go back to a larger screen it looks like the navbar isn't repaired; possible the function that adds the class isn't being called again?
Finally, we probably do want the hero to be in a centered container so it doesn't leave the positioning from the rest of the application (e.g. it stays on the left while the rest of the content is centered):
It seems like tailwind should handle most of this stuff; doesn't their grid system handle a lot of this behavior automatically?
| @@ -0,0 +1,9 @@ | |||
| /* Toggles between adding and removing the "responsive" class to topnav when the user clicks on the hamburger icon */ | |||
| export default function myFunction() { | |||
There was a problem hiding this comment.
I recommend that we use a more descriptive function name; perhaps toggleResponsiveClass?
There was a problem hiding this comment.
That sounds good!
| <img | ||
| className="w-full absolute inset-y-0 left-0 lg:h-full lg:w-full lg:max-w-none" | ||
| src={footer} | ||
| alt="Illustration of a blue lighthouse on the left hand side." | ||
| /> |
There was a problem hiding this comment.
I would prefer to make this a background image and then use the same wrapper effect with tileable blue waves as we did in the header. Can we create a story to do that here?
There was a problem hiding this comment.
I was planning on creating a story for this. Edwin said that he'll create a tileable blue wave for the footer. Once that's ready, I'll update the footer.
|
I can make the text smaller and add some margin to the side. When I write the style in the className, that’ll be for the xs screens and then I can adjust for screens that are bigger. Edwin provided a tileable wave to use for the for header. I'll create a story to add it along with placing the hero within a container. Tailwind's grid system should handle a lot of the positioning. I'll take a look through the docs again. |
|
The header text has been amended for smaller screens so that it's now a little smaller and some spacing was added. I'll have to update the state for the mobile nav's state and create stories regarding the hero image and making the footer a background image. |
Scope of changes
Changes to form, header, and navbar to make landing page responsive. Added logo to navbar.
Additional changes to make: Prevent H2 text in header from going below the element. Improve footer's responsiveness.
Fixes SC-9809
Type of change
Acceptance criteria
Describe how reviewers can test this change to be sure that it works correctly. Add a checklist if possible.
Author checklist
Reviewer(s) checklist