Mikas business site week 3 #380
Conversation
There was a problem hiding this comment.
Nice job with your business site, Mika! 🏃 You have a lot of files in your repo, so I'd suggest to remove all the ones you are not using. I started looking at demo.html before I realised it was a demo for the fonts 😅 Can be good to keep in mind going forward, to keep things clear and clean.
HTML
- The
brtag is a bit outdated, try to achieve the same thing using flexbox - I see a commented-out video tag for the hero section. It’s good practice to remove any unused code from your production files
- I see you've added "What's your main sport?" as one of the values in the select - great way of adding a placeholder! Just make sure to add the
disabledattribute so that the user can't pick that option
CSS
- You’ve done a great job using CSS variables ⭐ Just make sure to remember to use them where applicable!
- Make sure your paths are correct. I think to reach the fonts from the CSS file you need the double dots to "move up a level" from the code folder - i.e. "../Fonts". And on this topic, rename all files and folders to lower case kebab-case.
Really good job overall with this project, just keep these things in mind as you progress 🚀
There was a problem hiding this comment.
Great job! Your code works without any issues, and it looks really good on both mobile and desktop. I like the design, but one thing I would like to add is make (P) a bit more responsive on mobile devices with screen widths between 320 and 333 pixels. See image attacted image.
Overall, you have a nice structure for the HTML and CSS. It’s easy to follow and understand. Also noticed that you have a lot of code related to fonts, and I’m curious about what it does. Well done!
There was a problem hiding this comment.
Accessibility feedback
Your form and the rest of the interactive elements are possible to navigate with keyboard, super! Your form have all the labels it needs too. You’ve used semantic elements, which means you get a lot of accessibility ”for free”, but you’ve also made use of aria labels to provide additional descriptive meaning to some of your elements.
I see you’ve worked on increasing the contrast for your colors this week, which is great - but the text "Get real with clinically tested isothiocyanates from 🥦"
is still really low contrast, so I’d suggest changing that to make it more accessible. Another thing to think about is the outline for form elements. The outline is there for a reason, so don’t style it away with too low of a contrast - it helps the user see what’s in focus 👀
Changes Requested
- Sufficient contrast for all text plus form elements' focus state
No description provided.