Skip to content
OverviewSolutions
13
Comments
8
Muhammad Ahmad Hassan
@hassanahmadp

All comments

  • Muhammad Ahmad Hassan•340
    @hassanahmadpPosted over 3 years ago

    Hello Fadi,

    Hope you are alright... You did a great job, however, You lacked at some points. Which near to me is due to a lack of theoretical knowledge... I recommend you to have a strong search on the following concepts because these are very crucial:

    1. Z-index

    2. Z-index scoping ( This is hard to find but solves a lot of problems )

    3. Flex-box

    4. Grid

      Moreover, you have used accessive media queries which is not an efficient way and will take a lot of your time. I recommend following along with some youtube channels and see how they write code... What I can see is you are trying to run before trying to walk, and in this way, you are getting a hard time running...

      I recommend slowing down a bit and try to understand the crucial concepts of HTML and CSS...

      I hope this helps, Happy Coding!

    0
  • Muhammad Ahmad Hassan•340
    @hassanahmadpPosted almost 4 years ago

    You did a great work!

    just add transition: all .2s ease; to the button so that it nicely animates instead of just jumping between colors...

    other than that... your work was great.

    0
  • Muhammad Ahmad Hassan•340
    @hassanahmadpPosted almost 4 years ago

    You did it well!

    Although it is not pixel perfect, but with respect to UX principles, Your work is more user friendly... For a second I got confused, that which one is the design and which one is the solution.

    There was one place where I felt that this could be a little bit better, where on the menu slide (mobile view) there is an overlay behind the menu, whereas in the design there is a box-shadow with a border-radius.

    I hope this makes sense to you.

    1
  • Muhammad Ahmad Hassan•340
    @hassanahmadpPosted over 4 years ago

    Well done Shafiq!

    You have done a great job. Here are a few suggestions for you:

    • You should use only those font-weights which are recommended in the style-guide.md.
    • For adding a background color you don't need to make a separate div and give it a background-color (correct me if I am wrong).
    • Give the body a height: 100vh; and width: 100vw; and there is no need to have a div classed background and if it is necessary then you can give it height: 100%; and width: 100%;.

    Other than that everything is done perfectly... I have also done this project... Here is the link to that have a look and give me suggestions too.

    1
  • Muhammad Ahmad Hassan•340
    @hassanahmadpPosted over 4 years ago

    Weldone Agata!

    You have done a great job! Let me try to answer your Questions:

    • Firstly, the is no need to have multiple HTML files for such a small project. One HTML file is more than enough. As far as CSS is concerned, I have you writing margin: 0; and padding: 0; and there are some other repetitions as well. What I do for that is I write the following piece of code at the start of my CSS * { margin: 0; padding: 0; box-sizing: border-box; } This makes you skip many errors in the long run.
    • As covered in the previous answer, you should use universal selector (*), also SASS is a great tool to structure and organize code. I will recommend you watching this short introductory video about SASS.
    • My personal opinion for you is to learn SASS.

    Tell me which code editor you use so that I may suggest more things...

    3
  • Muhammad Ahmad Hassan•340
    @hassanahmadpPosted over 4 years ago

    Great work is done Janne!

    As you asked about using positioning, it is perfectly fine to use it. However, I found some issues with the mobile view as the main SVG part does not show properly, for that I suggest using fixed height in this case (although I discourage using fixed heights)... also there is one minor issue too that is there is no alt attribute for the image used... you need to work on the drop-down transition too...

    Other than that you have done a great job...

    2
  • Muhammad Ahmad Hassan•340
    @hassanahmadpPosted over 4 years ago

    Great work done!

    I will suggest you to focus more on design and try to make it as close as possible to the design provided in the source file. Otherwise, every thing is great. Please let me know whether you have used BOOTSTRAP or not?

    Regards,

    1
  • Muhammad Ahmad Hassan•340
    @hassanahmadpPosted over 4 years ago

    Hi Mexitaco!

    You have done great job. However, as you asked, if you want to write less code you can use SASS or SCSS instead of CSS.

    Thank You.

    0