@ApplePieGiraffe
Posted
Greetings, Roc Tanweer! 👋
It's good to see you complete another challenge! 😀 Great job on this one! 👍 Overall, your solution looks good and is responsive and it's wonderful that you learned lots of new things (I always do when completing challenges, as well)! 😊 And I like the animation of the mobile menu icon and the custom attribution that you added to your solution! 😄
A few things I'd like to suggest are,
- Adding
overflow-x: hiddento thebodyof the page (or something similar) to prevent a horizontal scroll bar from appearing along the bottom of the page when the image changes in the "Features" section. - Adding a
max-widthto the header of the page in the desktop layout to ensure that it isn't too wide on extra-large screens. - Using the
<button>element (rather than a<div>) for the hamburger menu icon (since it is button and that will improve the accessibility of your solution). - Perhaps taking another look at your class names. If you're trying to use BEM, I believe there should only be one element per classname (in other words, something like
block__elementinstead ofblock__element__element), so you might want to simplify some of your classnames a little. I don't think classnames have to reflect the actual structure of your HTML unless that makes things confusing, so you might be able to do things likeheader__linkinstead ofheader__nav__links__link. - Considering taking a look at this StackOverflow page (as it might help you clear up those few errors on your solution report).
- I noticed you created the accordion in this challenge with HTML and JS (and it works well, kudos!) but if you're interested, you can create accordions with just HTML by using the
<detail>and<summary>elements. It's pretty nice and easy, and you can learn more about it from this helpful MDN page.
I hope those tips help. 🙂
Of course—keep coding (and happy coding, too) my friend! 😁
@RocTanweer
Posted
@ApplePieGiraffe Thank you for your words..! I really admire you..!
- There is some kind of bug which is the reason for that overflow. When it shows, if you click on the ham menu or submit the form or click on the attribution icon, overflow will be gone(There shouldn't be any overflow because nothing is overflowing). It started happening when I added animation to feature section and hero section...
- I will add max-width right away.
- I did
header__nav__links__linkfor SCSS nesting...I saw your words with grace and now I know how to fix that - I will look at Stack Over flow [Edited: I looked at their answers, but I couldn't understand. If I add a text in an icon link, then it will be visible. If I hide it
visibility:hiddenthen it will take space. What should I do?] - I will try making another accordion with your advice
Happy coding too..!
@ApplePieGiraffe
Posted
@RocTanweer
Alright—that sounds good! 👍
As for the icons that are wrapped in anchor tags, you could add a <span> inside the anchor tag (next to the icon) with some text that describes the purpose of the link and then visually hide it from users with a sr-only class (you can easily copy-and-paste and sr-only class from the Internet), like this,
<a href="/">
<i class="fa fa-icon"></i>
<span class="sr-only">Description</span>
</a>
You could also add aria-hidden: true to the icons so that screen readers don't get confused about it or anything. 🙂
Looking forward to your next solution! Cheers! 🙌
@RocTanweer
Posted
@ApplePieGiraffe Thank You..!!!
