Yari Morcus• 480
@YariMorcus
Posted
Hi @Mibenin,
First of all, congratulations on completing this challenge! I took a look at your design, and it looks pretty good.
However, I do have some feedback I want to share with you so that you can become even a better developer than you already are.
My feedback is divided into sections to make it more clear.
Mobile
- I think you forgot to add whitespace on both the left and right sides of the card component. It is currently clung against both sides.
- The 'Proceed to Payment' button should be a little higher according to the design.
- The text within the 'Proceed to Payment' button should have a larger font size.
Desktop
- Your 'Change' button has been made up with
<a>. You should be using<button>instead according to the <a>: The Anchor element page on MDN. The anchor element is something you should only use when you navigate to a real hyperlink (which wouldn't apply in this case, because if it was a functional application, it would probably bring up a dropdown menu). - Your 'Cancel Order' button has been marked up with
<p>. You should be using<button>instead, because the latter conveys important information to assistive technologies such as screen readers. You did use<button>for the 'Proceed to Payment' button, so that is good. - The text color of the 'Cancel Order' button does not change on hover, and
cursor: pointer;is not shown either. This has probably to do with the prior point. - The 'Cancel Order' button should have
font-weight: 700;if I am seeing this correctly in the design (it is a little thicker than the 'You can now listen to.....' text). - You have both a horizontal and vertical scrollbar on desktop view. Is there a reason why you applied
overflow: scroll;to.container?
I also want to give tips to you based on looking at your code.
- Try to be as specific as you can be in the lang attribute on
<html>. In this case:lang="en-us". This is important for not only search engines but also browsers. Search engines for example, know what language your document is in and in which language your page should be ranked. Browsers on the other hand, know better what dictionary has to be applied to make sure the right grammar and spelling checks are applied. - The content within
<title>should always read from specific to general. In this case, that would be "Order summary card | Frontend Mentor". This is important for SEO reasons, because search engines always truncate text that is longer than a specific amount of characters (I thought this was between 155 and 160). In this case it is a short title so it should not cause any problems, but try to make it a habit. - Try to use a little bit more of the semantic HTML5 elements, such as
<main>,<article>and<section>. This is important for assistive technologies because it communicates better what the role of a specific element is. A div or span for example, does not communicate any meaning or role, which is important for people using these technologies.
I could give you some more feedback and tips but I think this should do it for now.
However, I hope you can do something with the feedback and tips I gave you. If you have any more questions, feel free to ask me.
If I made a mistake somewhere in this post, feel free to correct me and keep building awesome things 😄
Marked as helpful
0
