@ApplePieGiraffe
Posted
Hello, Kaho! 👋
Well done on this challenge! 👏 Your solution looks great! 🙌
A few things I'd like to suggest are,
- Adding a tablet layout between the desktop and mobile layouts if you're up for the challenge. It would be a nice thing to do to get a little more experience with layouts and media queries and improve the design of the solution. 😉
- Considering not using multiple
<h1>
elements in a single page (since, at the moment, it's most often recommended that there be only one<h1>
element on a page). You can simply use other less-important heading tags like<h2>
or<h3>
. - Perhaps using the
<article>
element for the testimonial cards (sine they are self-contained elements) to bump up the semantics of your HTML a bit. - Not using ID's for styling things in your CSS (since that's not really their intended purpose and can potentially make things difficult due to their high specificity). Simple classes should be enough. 🙂
You've split your Sass into separate files and I noticed you're using things like mixins, which is nice! 👍
Keep coding (and happy coding, too)! 😁
@shiv-chan
Posted
Thank you for your comment! I did a code refactoring based on your suggestions although I haven't merge it to the main branch. Thanks again!
@ApplePieGiraffe
Posted
@shiv-chan
No problem! Glad to help! 😀
Oh, and hey, if you found these tips helpful, an upvote would be appreciated! ;)