Mai & Martin - Worky Design Handoff #50
Conversation
Footer section 2
Learn more section 2
Social media icons 2
Co-authored-by: Mai K <[email protected]>
There was a problem hiding this comment.
Hello Mai & Martin, big congrats for completing your week 12 project!Great job ⭐ The site looks very close to the design and followed our requirements. How did you like Tailwind? Seems like you tackled it in the right way and implemented a great result! Super cool you localised the page. Now the design is not pixel perfect, but that's okay, here's a little things that need to be fixed to achieve the basic requirements:
- On tablet the footer layout doesn’t match the design
- Accessibility: you scored 89, should be at least 95
- Keep an eye on responsiveness on small screen (320px width)
Please fix these little things and you're good to go 🚀 Almost there!
There was a problem hiding this comment.
We noticed that all your text is located in a JSON file; that made it a bit hard to review/read but we also understand that it was made for the translation goal; which we found really cool 🚀.
Overall your code is clean and well structure. Well done !!!
//Alma & Etna
| export const App = () => { | ||
| return <div>Find me in src/app.jsx!</div>; | ||
| const [locale, setLocale] = useState("en"); |
There was a problem hiding this comment.
Amazing feature!! We did not know the change-language button works until we saw your code!
//Alma & Etna
| return ( | ||
| <div className="border border-black desktop:border-none desktop:drop-shadow-none bg-background-neon-green rounded-lg drop-shadow-card mx-4 my-12 tablet:my-18 tablet:mx-6 flex"> | ||
| <img | ||
| className="w-[100px] tablet:w-[120px] desktop:w-[160px] self-center ml-2 tablet:ml-4" |
There was a problem hiding this comment.
Good approach on adding your own widths to better follow the design.
//Alma & Etna
| const [isMobile, setIsMobile] = useState(false); | ||
|
|
||
| useEffect(() => { | ||
| const checkIfMobile = () => setIsMobile(window.innerWidth < 360); |
There was a problem hiding this comment.
We do not quite follow how this function works, but if it is for responsiveness on the icons; great approach to make it work!!
//Alma & Etna
| checkIfMobile(); | ||
| } | ||
|
|
||
| window.addEventListener("resize", handleResize); |
There was a problem hiding this comment.
Seems like the icons are a bit too big for the smallest screen; this section gets bigger than the view port, you have to scroll horizontally.
//Alma & Etna
| @@ -0,0 +1,13 @@ | |||
| import PropTypes from "prop-types"; | |||
There was a problem hiding this comment.
We see that you have two button components; maybe you could have made one button component reusable ((:
//Alma & Etna
| {data["smallSubheading"]} | ||
| </span> | ||
| <span className="hidden md:block"> | ||
| {data.subheading.split("\n").map((line, index) => ( |
There was a problem hiding this comment.
Very creative ways of targeting the text display situation.
//Alma & Etna
| <React.Fragment key={index}> | ||
| {" "} | ||
| {line} | ||
| <br /> |
There was a problem hiding this comment.
We heard that
is outdated and there might be accessibility issues for it :(
//Alma & Etna
Netlify link
https://worky-design.netlify.app/
Collaborators
[maikanetaka]