Damilola Olanrewaju
@Dami-MooreAll comments
- @marianaceciSubmitted over 2 years ago@Dami-MoorePosted over 2 years ago
Hi Mariana! Great work and congrats on completing this project. Nicely done.
Just a few comments based on the accessibility and HTML issues.
-
Image tags require an "alt" attribute, of which it's value is set to a description of the image. This will aid other users, for example, those who are visually impaired, to find your site more accessible. A screen reader would identify that alt attribute. Also, in the case that the site fails to load, maybe due to poor internet, the alt text will be displayed in place of the image, still giving an idea of what the image displays.
-
Using headings is also a bit tricky. Instead of an h3 tag, use an h1 but maintain the styling in your style.css.
Hope you find this helpful.
Happy coding!
1 -
- @marianaceciSubmitted over 2 years ago@Dami-MoorePosted over 2 years ago
Hi Mariana! Great work on this challenge and congrats on it's completion. A nice responsive design.
A comment on Accessibility:
- You need to wrap your code within semantic tags. For example, you could wrap the whole code after the <body> tag with a <main> tag.
Marked as helpful1 - @Jaybabson007Submitted over 2 years ago
Please how and where should I use the mobile and desktop width provided in the style-guide.md file, I was able to achieve this design without using it
@Dami-MoorePosted over 2 years agoIt's working now.
Great work, great coding. The desktop and mobile widths given within the style guide are to help with the responsive designs. So, at 1440px, you have the desktop view and at 375px, you have the mobile view. You'll need to understand how to use media queries to get this (I'm sorry if I wrongly assumed that you don't ).
On Accessibility and HTML issues:
- Section tags are usually within the main tag not the other way round as in your code.
- It's also preferable to have your image tags contained within divs.
I hope this was helpful? Happy coding!
0 - @Jaybabson007Submitted over 2 years ago
Please how and where should I use the mobile and desktop width provided in the style-guide.md file, I was able to achieve this design without using it
@Dami-MoorePosted over 2 years agoHello Joel! Congratulations on completing this project. To give feedback as requested, we'll need your code. However, on clicking to view your code, it seems like you haven't uploaded your code on your Github. I can only see the read.me file. Please check.
Thank you. Happy Coding!
Marked as helpful0 - @KohseyPowerSubmitted over 2 years ago
I fixed the issues that @Dami-Moore wrote to me.
Feedback are welcome.
@Dami-MoorePosted over 2 years agoHello Koshey! Congratulations on completing this project and that's great progress.
I have a few comments.
- In your style.css file, it's my opinion that adding the ".menue ul li button:active" in a way conflicts with the click event in your script.js file. I think you can safely remove this portion or comment it out.
- Also, the "cursor: pointer; " within the ".menue ul li button" selector in your style.css can also be removed and put instead within the ".menue ul li button:hover"
- Also, directly styling the button element, is why the hover effect on the button element still works after a button is selected. Instead, you can use a class and style that class initially, then in your DOM, you can remove it and add the new class ".selected". This will ensure that every styling associated with that previous class is removed and only stylings within ".selected" will display.
On accessibility: The issue noted is semantics. There should be landmarks to describe each parts of your code. For simplicity, you can wrap all your code, after the body tag, with a main tag. There's more to comment on as regards semantics here, but to keep it simple, that's that.
I hope this was useful and of help to you.
Happy Coding!
Marked as helpful1 - @Katja721Submitted over 2 years ago
Any feedback appreciated.
@Dami-MoorePosted over 2 years agoHi there! I've been trying to view your code and keep getting an error message. Kindly check the link to see if there's any problem.
Marked as helpful0 - @ketannegiSubmitted over 2 years ago
I'd love to hear your lovely feedback.😊😊
@Dami-MoorePosted over 2 years agoHi @katannegi! Great attempt. The site is quite responsive.
I have a few comments for your html code.
It's not semantically right to have multiple headers. So, in this challenge, since there isn't like a main header, it can be left out. Instead, you can contain each section (box) within a <section> tag, then within each <section>, wrap each segment in a <div> and then wrap the whole code after the <body> tag with a <main> tag.
The attribution section can be wrapped in a <footer> tag instead of using a <div>.
This will greatly improve accessibility.
I hope this was helpful! Happy coding!
0 - @ogheneCodesSubmitted over 2 years ago
Kindly rate this and give me any critical feedback so I can keep on improving, Thank you.
@Dami-MoorePosted over 2 years agoGood attempt! @OgheneCodes. A few suggestions. For the layout, you can remove "margin-left: 5%" from the seconddiv and thirddiv classes in your style.css
Concerning accessibility issues, the <img> tag will require an alt attribute which will enable your site to be accessible to all kinds of users. The value for the alt attribute is a description of the image, that can be accessed by, for example, someone with visual impairment using your site.
Also, headers also help accessibility, so you can change the <p class="title"> tags to h1 tags but still maintain the styling.
Your site is also responsive, only that the buttons are a bit off position on mobile view, I'm thinking that might have been an oversight.
For me, it took a while to really come to understand how to use semantics in html (I'm still learning) and I know it really helped my coding.
I hope this was helpful! Happy Coding!
0 - @anoshaahmedSubmitted almost 3 years ago
I used desktop-first approach for this, and now I know for sure that mobile-first approach is best.
Please let me know what I can do to improve.
@Dami-MoorePosted almost 3 years agoYour works are really awesome. You're really good at your stuff!! How do your designs match so perfectly? It's not like they give the exact sizes in the style guide. How do you do it? Or are you just a perfectionist and sit for hours trying out different sizes till you get an exact match? 😃
Marked as helpful1