Skip to content
OverviewSolutions
46
Comments
104

Mohammed Fakih

@javascriptor1Dammam1,590 points

Web Development - HTML - CSS - Javascript

I’m currently learning...

Javascript - React

Latest solutions

View more solutions

Latest comments

  • Mohammed Fakih•1,590
    @javascriptor1

    I have updated my solution to this easy challenge. Original solution a year ago had an overflow for mobile design.

  • Mohammed Fakih•1,590
    @javascriptor1

    UPDATE

    I have fixed the problem in my code. Its working fine now.

  • Mohammed Fakih•1,590
    @javascriptor1

    UPDATE.

    I have solved the challenge again using React. However, there is a bug in my code.

    When the page first loads, and with first click , the question toggles correctly. But after that, I need to click twice on each question to toggle.

    I also feel my code is not perfect and could be done better. I would appreciate any feedback on my solution.

    Thanks

  • Mohammed Fakih•1,590
    @javascriptor1

    Hi Vanessa,

    Congratulations on completing your second project. Excellent job. Here are some remarks which I hope you found helpful:

    HTML

    • You have done a good job using semantic HTML elements. There is one tag in the ingredients section which should be change from ul to 'ol' ;

    <ul class="instructions__list">

    As this is an ordered list , the ol should be used.

    • For alt text related to images inside HTML , and as per W3C website , you should avoid "Superfluous information in the text alternative. Usually, there’s no need to include words like “image”, “icon”, or “picture” in the alt text. People who can see will know this already, and screen readers announce the presence of an image".

    • The last row in the Nutrition section table should not have a border bottom. You have to remove the border-bottom by using .nutrition__table tr::last-child selector and assign value none to border-bottom

    CSS

    • The main container should be a little wider. This affects some content and causes extra lines for some p elements like the one under h1 tag. Instead of 2 lines like the design, your paragraph spans into 4 lines.

    • For responsiveness, you said in a reply to one comment the following :

    "I used 1440px cause was the number in the guide styler!"

    When I started learning about responsiveness, and start doing FEM challenges here, I got confused just like you about the two designs ( 375 & 1440px) which appear in all challenges style guides.

    The design folder for all challenges is shipped with at least 2 photos :

    1- For mobile devices ( width is 375px).

    2- For desktop computers ( width is 1440px).

    However, your solution should be responsive even for devices that fall between these 2 sizes like 500px, 800px, and 1200px.

    These 2 designs are working as a guide for you on how the mobile design should look like and how the desktop design should look like.

    For example, in this challenge, The image on top should take full width (100%) on mobile devices. So you should make your design considering this and set a breakpoint as to where to break into desktop design.

    If we check your solution again , try to view the design on 799px using dev tools in chrome, then go up to 800px where you set a breakpoint for desktop design.

    You will see a big photo until 799px then a small container when you switch to desktop design( you set it max @ 35% of screen width). Sô, here is a problem. The screen is 800px and the container is only 35% wide of the screen which is very small.

    So, To code it right, whenever you break into the desktop design, the page should look like the same one provided in the design folder for desktop view.

    I hope it's clear now for you.

    • There are some declarations in CSS which have no effect as they are default values. Examples of this are the following ( see the last commented two declaration) :
    .table__infos {
    font-family: var(--secondary-font);
    color: var(--wenge-brown);
    /* font-size: 16px; */  
    /* font-weight: 400; */
}

    When you finish styling, try to clean up your CSS files by removing all declarations which have no effect.

    Best of Luck and keep coding.

    Regards,

    MKF

    Marked as helpful
  • Mohammed Fakih•1,590
    @javascriptor1

    Hi Nouran,

    Congratulations on completing the challenge. Here are some notes which I hope you find helpful in improving your code :

    HTML

    • Use semantic elements like main instead of div. When you create a web HTML page, you are authoring a document that should convey a meaning. So always think semantically and check what content each part of the document contains and choose the semantic element accordingly. Here are a few examples to make this clear ;

    1- use the main tag for the main part of the document instead of div. A main landmark identifies the primary content of the page. You can read more on main tag here

    2- use the time tag when your document contains a date/time.

    3- use the abbr tag for any abbreviations like HTML & CSS in your document.

    CSS

    • The font required for the challenge is available with downloaded resources under the assets folder. Try to use font-face instead of importing the font from the Google fonts website.

    -For mobile view @ (375px), there will be no margin from the right/left of the card. This is because you set a fixed width for the card(375px). The width should be less than 375px for mobile design so the card will have the required space from right/left. When you reduce the width to say 334px , the image needs to have width:100% so it does not get outside of the card.

    Keep coding and good luck.

    MKF

    Marked as helpful
  • Mohammed Fakih•1,590
    @javascriptor1

    Hi Tringa,

    Congratulations on completing the challenge. Here are some notes which I hope you find helpful in improving your code :

    HTML

    • Use semantic elements like main instead of div. When you create a web HTML page , you are authoring a document that should convey a meaning. So always think semantically and check what content each part of the document cotnain and choose semantic element accordingly. Here are few examples to make this clear ;

    1- use main tag for the main part of the document instead of div

    2- use the time tag when your document contains a date/time.

    3- use abbr tag for any abbreviations like HTML & CSS in your document.

    • Even when using aria-label, you still have to use the alt attribute. An aria attribute can be used as an additional text alternative but doesn't replace the requirement for an alt attribute.

    CSS

    • For the font family, you have imported the font from Google and also used the locally hosted font in the assets folder. One should be enough unless you did this intentionally.

    • For font-weight , I can see you have used bold which is equal to 700. You also used 900. The requirement is to use 600,800 only.

    • The styling for body tag is overcomplicated and I guess you should consider re write it again. Here is a very simple and straightforward styling for the body tag if you want to place content in the middle of the page :

    display: flex;
justify-content: center;
align-items: center;
min-height: 100vh;

    -For mobile view (375px), the h1 content breaks into new line. Reduce the font-size used for mobile view.

    Keep coding and best of luck.

    MKF

    Marked as helpful
View more comments