Skip to content
OverviewSolutions
23
Comments
14

Yari Morcus

@YariMorcusNetherlands500 points

An eager to learn student with a future vision in the Front-end development career.

I’m currently learning...

At this moment I am learning React with the help of a course on Udemy. As a result, I will not be creating a project until I made enough progress in this course.

Latest solutions

View more solutions

Latest comments

  • Yari Morcus•500
    @YariMorcusPosted about 1 year ago

    Hi Tuğçe,

    First of all, you did a great job at replicating the challenge.

    I do however have some tips for you to become even a better Front-end developer.

    Feedback and tips

    1. Your share container causes an overflow of the body on mobile. As a result, a horizontal scrollbar appears. One way to solve this is to show the share container underneath the author container (in your case, the div.footer element). The challenge however indicates that it is acting as an overlay over the author container.
    2. I wouldn't be using an img as a button by attaching JavaScript to it. Instead, I would be using the button element to which you can add the img element, or add the image with a CSS background image. Your image buttons are not accessible for keyboard navigation at the moment (you can try this yourself by pressing the tab key). In other words: people who are not capable of using the mouse due to a handicap (such as dexterity problems) cannot use your buttons. I do not want to exaggerate, but this is a serious issue concerning Web Accessibility.

    I hope you can do something with the feedback I gave you. Do not see this as something you should do right now. You can also decide to apply it for a later project. I'm just pointing some things out that I noticed.

    If you have any questions, you can let me know and I will try to respond as quickly as possible.

    And last but not least, keep building awesome projects :D

    0
  • Yari Morcus•500
    @YariMorcusPosted about 1 year ago

    Hi Giovani,

    There is no need to use JavaScript if you can avoid it. By implementing the dropdown menu with CSS only, you made sure that even people who do not have JavaScript for whatever reason can still use your menu.

    Your users will thank you for that ;)

    1
  • Yari Morcus•500
    @YariMorcusPosted over 1 year ago

    Hi Aaron,

    First of all, I think you did a great job coming up with a solution for this challenge. It looks exactly like the design, it is functional and also responsive.

    There is one thing I noticed and that is the following.

    I can press the submit button without selecting a number. This results in the message You selected out of 5 (missing the number). You could give it a default rating number (which you should not do in a real application unless clearly indicating what this number will be), or you could also disable the button and enable it when the user selected a rating.

    It appears you get to choose haha.

    I hope you can do something with this feedback. If you still have questions, let me know.

    If I made a mistake somewhere in this post, feel free to correct me and keep building awesome things :D.

    Marked as helpful
    0
  • Yari Morcus•500
    @YariMorcusPosted over 1 year ago

    Hi SuperJulia2024,

    First of all, I think you did a great job coming up with a solution for this challenge. It looks exactly like the design and is also responsive.

    Here is my answer of your question about margin-tops and bottoms. I currently do not have time to also answer your second question about pixels and (r)em. I will come back to this solution to answer this question as well as soon as I got more time (and if I did not forget it 😳).

    1. I read somewhere (and I kind of share the same opinion) that you should always apply a margin bottom to elements. This has to do with a concept called 'margin collapsing'. If you start mixing margin tops and bottoms of elements and you do not understand this concept, then it can become quite hard to calculate the real distance between elements. This is because those margin tops and bottoms start to collide with each other. For example: if you apply a positive margin-bottom of 10px to element a, and a positive margin-top of 20px to element b, the real distance between element a and b will be 20px (not 10). This is ofcourse a simple example but in a real big project this can become quite complex (and frustrating if people start mixing them). I personally only use a margin-right and left on a complete container to push it away from other surrounding containers (a container that is completely independent from others (Containers like these should be marked with <article> (here you got a free tip haha))). Concerning your solution: I personally would have placed the margin-bottom: 1.8rem of .current_price on the container with .price.

    Other feedback I would like to give you.

    Feedback

    1. Create a hover state for your 'Add to Cart' button with enough contrast between the enabled state and hover state. This gives the user a distinct indication that they are hovering over an interactive element. You can achieve this with an overlay with a lower opacity than your current background color (to make sure people are not distracted).

    I hope you can do something with this, but if you still have questions, let me know. Don't see it as something you need to do, but rather something for later (you are always free to correct your solution).

    If I made a mistake somewhere in this post, feel free to correct me and keep building awesome things :D.

    Marked as helpful
    0
  • Yari Morcus•500
    @YariMorcusPosted about 2 years ago

    Hi Túlio,

    First of all, I think you did a great job on the challenge. The component is responsive and almost looks like the design (you forgot to add a white background to the result2 section).

    The only thing I noticed beside the above is that your favicon is not working (due to an incorrect path, you can see this in the JS console).

    To give you an answer to your question

    You can change widths more easily by not only using percentages (which you have done already), but also by using CSS Grid Layout.

    Alternative method with Grid Layout

    1. On larger screens (use a media query for this): on div.card, use display: grid instead of flex, and use grid-template-columns: 1fr 1fr;. This creates a grid formatting context (grid container) in which you can create your custom layout. In this case, a two column layout. The two column layout is then achieved with grid-template-columns: 1fr 1fr;, which basically says: create two columns of the same size. 1fr 1fr is here equal to 50% 50%.
    2. Remove width: 100% on both section.result and section.result2.
    3. Remove width: 500px on <main> and put max-width: 500px; on div.card instead. I also advice this when you use flexbox. The main reason for it is because it is a componen, which should be able to be reused on multiple pages (without setting width: 500px: on <main> all the time, because this can break your entire layout if you have other things on the webpage as well (but this is something for later on).

    If you do the above, you will see that it will give you the exact same result.

    I you want to learn more about Grid, then you can follow a crash course (Traversy Media) if you'd like.

    Flexbox variant (which you currently use)

    Instead of putting width: 100%; on both div.result and div.result2, you could have placed flex: 1; on both div.result and div.result2.

    Flex: 1; says: create two flex items of the same size (50% 50% because you have two flex items in your div.card container (flex container)).

    I hope you can do something with this, but if you still have questions, let me know. Don't see it as something you need to do, but rather something for later (you are always free to correct your solution).

    If I made a mistake somewhere in this post, feel free to correct me and keep building awesome things :D.

    Marked as helpful
    1
  • Yari Morcus•500
    @YariMorcusPosted about 2 years ago

    Hi Kareem,

    Even though you had it difficult to make the website fit on both mobile and desktop, I must say that it still looks pretty good.

    The best way to solve this problem is by following the 'mobile first' principle. In short: you create your design for mobile first, then tablet, and then desktop. With this approach, you use the min-width media feature rule within @media (@media (min-width: 768px) e.g.).

    Another tip I have for you is to define your layout structure first, and when everything works correctly, you add the visual parts to your box (background color etc.). What I always do is adding a border around all my boxes, make sure the layout is correct, and then remove the border (which was temporarily used to identify the boxes).

    Other tips

    1. Try to use a little more of the semantic HTML elements HTML5 offers you for structuring your document. You can think of <article>, <section> and <header>. When you start using these semantic HTML elements more, you make your component even more accessible for people who use assistive technologies (such as screen readers), because theses HTML elements are recognized better than using the common 'div' or 'span' for example.
    2. For your 'Add to Cart' button: use the <button> element. One of the reasons for this can be found above this point, but the other one is because you cannot place focus on the button (which can be done with keyboard navigation (tab key)).
    3. Try to bring a little order in your stylesheet. Your element, class, and id selectors are all mixed up. A common practice is to place the generic styling (element) selectors at the top of your stylesheet, the more specific (class) selectors underneath it, and then the id selectors. Another way to organize your CSS is to bring order to your CSS declarations. A handy website I use as a reference is codeguide.co.

    I hope you can do something with the tips I gave you. Don't see it as something you need to do, but rather something for later (you are always free to correct your solution).

    If I made a mistake somewhere in this post, feel free to correct me and keep building awesome things :D.

    Marked as helpful
    0
View more comments