Skip to content
OverviewSolutions
11
Comments
9
Mohamed
@mohamed-benoughidene

All comments

  • Laharl•1,000
    @UrbanskiDevSubmitted about 2 years ago

    Hello, Frontend Mentor coding community. This is my solution for the Intro section with dropdown navigation.

    I had some trouble creating the responsive component (mobile phone), but I was able to utilize CSS Flexbox and a tiny CSS Grid. The dropdown is functional, and I also created a hamburger for the mobile section. I also included some transition and hover effects.

    I'm sure there are some improvements I could make, but I believe it's a decent result.

    I would appreciate your feedback on my work.

    Intro section with dropdown navigation HTML/CSS (Responsive)

    1
    Mohamed•270
    @mohamed-benoughidenePosted about 2 years ago

    hi Laharl great job completing this challenge. I checked your code and i noticed that you didn't use any ARIA landmark. aria landmarks are very important for people how use screen readers because they navigate to a section based on its HTML element or ARIA Landmark. and you can do that by using the role attribute for example in line 103 of your code you can add it like this.

    <div class="hero" role="main">

    hope you find this helpful🥰.

    1
  • Laharl•1,000
    @UrbanskiDevSubmitted about 2 years ago

    Hello, Frontend Mentor coding community. This is my solution for the Manage landing page using Mobile First approach.

    The development of this page was very chaotic for me. I did it a first time yesterday, but I encountered a lot of problems when working on the desktop part after finishing the mobile view. I had some problem with my HTML structure, and I decided to call it a day. Next day, I decided it was better to start from zero again, I learned a lot on what was wrong the first time, what I could improve and this is the result I have after 3-4 hours of work. I haven't done the javascript part because it wasn't my priority for this challenge. I feel like the final result is not too bad, I could refine it a little more, but with all the difficulties I've got, I'm happy with it for now.

    Feel free to leave me a constructive feedback on my work !

    Manage landing page using HTML/CSS/Flexbox

    1
    Mohamed•270
    @mohamed-benoughidenePosted about 2 years ago

    Hi Laharl great work completing this challenge but there is a mistake in your html its not right to do this

      <nav>
        <ul><li>Pricing</li></ul>
        <ul><li>Product</li></ul>
        <ul><li>About Us</li></ul>
        <ul><li>Careers</li></ul>
        <ul><li>Community</li></ul>
      </nav>

    there is no need to use UL element like this instead do this

     <nav>
        <ul>
<li>Pricing</li>
       <li>Product</li>
       <li>About Us</li>
       <li>Careers</li>
     <li>Community</li>
  </ul>
      </nav>

    I hope you find this helpful

    1
  • Fatima Zahir•40
    @Fatima-hub333Submitted about 2 years ago

    Hi everyone, kindly check my solution. Are the best practices (DRY) I followed in the solution or not? & any other suggestions plz?

    QR Code Component Challenge

    1
    Mohamed•270
    @mohamed-benoughidenePosted about 2 years ago

    Hi @Fatima-hub333 ✌ great job completing this challenge it looks that you didn't follow best practices in your html witch is not using a level-one heading

    it's best practice to start your component with h1 because Screen reader users can use keyboard shortcuts to navigate directly to the first h1. in this challenge It wouldn't matter a lot but when you work on a bigger project it's gonna make a bigger impact on the usability of your website, so you can just change h3 tag in line 17 to an h1.

    finally, I hope you find my feedback helpful if so please upvote me and mark it as helpful 😃.

    Marked as helpful
    0
  • Mohamed•270
    @mohamed-benoughidenePosted about 2 years ago

    Hi @OhSorrow ✌ great job completing this challenge it looks that you didn't follow best practices in your html witch is not using a level-one heading

    it's best practice to start your component with h1 because Screen reader users can use keyboard shortcuts to navigate directly to the first h1. in this challenge It wouldn't matter lot but when you work on a bigger project it's gonna make a bigger impact on the usability of your website, so you can just change h2 tag in line 14 to an h1.

    finally, I hope you find my feedback helpful if so please upvote me and mark it as helpful 😃.

    1
  • Arush•180
    @arushkumar05Submitted about 2 years ago

    Hi, this is my solution for the huddle landing page. I had some problem with spacing of the right text content and the left illustration. What should I use? Border box? Also how can I make the icon borders a complete circle. For facebook it seems a little cramped. Any feedback is welcome...thanks! :D

    Huddle landing page using flex

    2
    Mohamed•270
    @mohamed-benoughidenePosted about 2 years ago

    for your question how can I make the icon borders a complete circle? the easiest way is that you give a different padding values to every icon because they have different widths and heights . the odder way witch is a little bit complex is that you go to figma and use font awesome icon plugin and add the circle there then export it as SVG then you have to use it as SVG element and edit it using CSS.

    i created this example on codesandbox :

    https://codesandbox.io/s/upbeat-bhabha-k5rlmy?file=/index.html

    for me I prefer using the first method because its the easiest and the fastest just check my solution for this challenge and you will find that I used a different padding values.

    finally, I hope you find my feedback helpful if so please upvote me and mark it as helpful 😃.

    Marked as helpful
    1
  • Mohamed•270
    @mohamed-benoughidenePosted about 2 years ago

    hi I checked your code and I noticed that you have 8 ACCESSIBILITY ISSUES all of them are because you didn't use ARIA LANDMARKS. for example in your code in line 11 you can make it like this

    <div className="container" role="main">

    aria landmarks are very useful for users of screen readers so it's best practice to use them. I hope you find my feedback helpful if so please upvote me and mark it as helpful 😃.

    Marked as helpful
    0
  • Mohamed•270
    @mohamed-benoughidenePosted about 2 years ago

    I noticed that you have 1 html ISSUES in button tag line 26 . you can't use p element in a button. just do this:

    <button type="button"> <img src="images/icon-cart.svg" alt="cart icon"> Add to Cart </button>

    I hope you find my feedback helpful if so please upvote me and mark it as helpful.

    Marked as helpful
    0
  • t-rieu•20
    @t-rieuSubmitted over 2 years ago

    I am not sure if the way I wrote my CSS for the SVG that overlays the image on hover is the most efficient. Any suggestions and recommendations for it are welcomed. Feedback on any other areas of my HTML and CSS codes is also highly welcomed.

    NFT Preview Card

    2
    Mohamed•270
    @mohamed-benoughidenePosted over 2 years ago

    hello t-rieu✌

    -first I noticed that you used 3 identical properties witch is display in the class named overlay line 26,28,31 you can write it ones because only the last one will apply in your case display: flex;.

    -for the overlay I used a different approach instead of wrapping only the svg inside a div i wrapped both the card img and the overlay div containing the svg inside one div.

    -for the overlay style i didn't use display: flex; like you did instead I did this background cyan , position absolute opacity 0, z-index 99, top: 0, left: 0, on hover state just opacity 1.

    • you can check my solution for that just head to my profile. I hope you find my answer helpful if so please upvote me and mark it as helpful.
    Marked as helpful
    1
  • Luyanda•40
    @lmaqungoSubmitted over 2 years ago

    What is the best method of adding that background shadow effect that is behind the main NFT card? I struggled to replicate that so I just used the CSS box shadow property I also struggled with implementing the hover state for the actual NFT image(the cube). The brief design had a cyan overlay with an icon showing for the hover state. I just resorted to using the box shadow again. If anyone knows how to do that, your advice would be appreciated.

    NFT Preview Card Component

    1
    Mohamed•270
    @mohamed-benoughidenePosted over 2 years ago

    Hello Luyanda 👋

    -For your question What is the best method of adding that background shadow effect that is behind the main NFT card I really think that you are already in the best solution.

    -and for your question about the hover state you can do that by creating a div that's gonna contain your card image and a div(containing the svg) that gonna have a style of background cyan (use hsla colors), position absolute opacity 0, z-index 99, top: 0, left: 0, then add a hover state that gonna make the opacity 1.

    -you can check my solution for that just head to my profile.

    -I hope you find my answer helpful if so please upvote me and mark it as helpful.

    1