Conversation
smirrebinx
left a comment
smirrebinx
left a comment
There was a problem hiding this comment.
Great job! This was a large and somewhat complicated design to follow and you did really well with the little time you had to work on the project.
| return ( | ||
| <div> | ||
| Find me in src/app.js! | ||
| <Webpage /> |
There was a problem hiding this comment.
Good job on having a main app instead of having all of the mounting in App.js.
| import icon4 from '../images/Twitter.png' | ||
|
|
||
| // eslint-disable-next-line max-len | ||
| const Footer = ({ footerImage, title, link1, link2, link3, link4, link5, link6, link7, link8, link9, shortText }) => { |
There was a problem hiding this comment.
The footer only needs a bit of styling with margins and maybe flex-box or grid and then it will look almost exactly like the design. Good job!
| import icon4 from '../images/Twitter.png' | ||
|
|
||
| // eslint-disable-next-line max-len | ||
| const Footer = ({ footerImage, title, link1, link2, link3, link4, link5, link6, link7, link8, link9, shortText }) => { |
There was a problem hiding this comment.
I like that you used props for the footer content, that makes it reusable.
| import Food from '../images/Food.png' | ||
| import Hamburger from '../images/Hamburger.png' | ||
|
|
||
| const Header = () => { |
There was a problem hiding this comment.
The header in desktop mode looks nice, just a bit of styling for the navbar is needed, and maybe add some margins.
| import React from 'react'; | ||
| import '../index.css'; | ||
|
|
||
| const Hero = ({ title, subtitle, buttonText, onClick, heroImage }) => { |
There was a problem hiding this comment.
The hero section looks nice, just a bit of styling is needed for the button :-) According to Wave there is too little color contrast between the background image and the text but to me, it seems fine.
| import React from 'react'; | ||
| import '../index.css' | ||
|
|
||
| const Plan = ({ title1, title2, plans, buttonText, finishingText, onClick }) => { |
There was a problem hiding this comment.
This is a good start, just some styling and flexbox or grid needed to complete the design :-)
| import img6 from '../images/Program-7.png' | ||
|
|
||
| // eslint-disable-next-line max-len | ||
| const Program = ({ title, img1Title, img2Title, img3Title, img4Title, img5Title, img6Title, buttonText, onClick }) => { |
There was a problem hiding this comment.
The Program section looks good, you just need a flexbox or grid to make it look more like the original design.
| backgroundImage: `url(${backgroundImg})` | ||
| }; | ||
|
|
||
| return ( |
There was a problem hiding this comment.
The Testimonial section looks good, you just need a flexbox or grid to make it look more like the original design.
|
|
||
| // import backgroundImage from '' | ||
|
|
||
| const WebPage = () => { |
There was a problem hiding this comment.
Good practice to mount everything here instead of the App.js.
|
|
||
| .header-section { | ||
| background-color:#7034E4; | ||
| height: 4rem; |
There was a problem hiding this comment.
Good choice on using rem instead of px for the CSS.
smirrebinx
left a comment
smirrebinx
left a comment
There was a problem hiding this comment.
I added a comment in the Hero.js.
| <div className="hero-container" style={heroBackground}> | ||
| <div className="hero-content"> | ||
| <h1>{title}</h1> | ||
| <h3>{subtitle}</h3> |
There was a problem hiding this comment.
Best practice is to not skip levels of headings. If you stick to the levels it makes it easier for screen reader users to follow along on the page. If you change the h3 to a h2 you would follow the levels of headings. If you want the h2 to be the same size as the h3 you can style it in CSS.
2 participants
This project is far from completed, I am doing the pull request so that the code can be reviewed and I intend to complete the styling and the responsiveness further on.
https://design-handoff-project-sow9.netlify.app/