Skip to content
OverviewSolutions
41
Comments
11
HashimAlSadah
@HashimAlSadah

All comments

  • Joshua Ajorgbor•210
    @JAjorgborSubmitted about 2 years ago

    This is the first site i made with sass, any tips or suggestions on how i can make cleaner and more efficient code would be appreciated

    RESPONSIVE BLOGR LANDING PAGE

    1
    HashimAlSadah•860
    @HashimAlSadahPosted about 2 years ago

    Hi @joshuaAj003. Congrats on completing this challenge, your Sass code looks great and I like the organization of the code where every section is placed into a separated file.

    I have noticed that you are using mixin for the gradients sometimes and sometimes you have them as variables, and some times you hard code the gradients directly, which makes you inconsistent. I suggest that you have the gradients as variables and not as mixin.

    I also notice that your code is deeply nested and that can confuse you and the reader and can lead to specificity issues (your Sass will be complied into CSS with too specific selectors). I suggest that you nest one level down or two at most.

    I also suggest that you make the screen break-points as variables, so they can be easily modified later if it is needed.

    I noticed that you are having @media (max-width: ...px), which means you are doing desktop first (correct me if I am wrong), which is not the best practice. It is usually mobile first.

    Anyway, I like your code and the work you have done and my suggestions do not make it any less impressive.

    Marked as helpful
    0
  • HashimAlSadah•860
    @HashimAlSadahPosted over 2 years ago

    Hi @Fahatmah, congrats on completing this challenge. For the background in the desktop view the image takes half the height of the screen and it fills up the whole screen in the mobile view. To implement this you need to do the following for the desktop.

    body{
    background-position: bottom center;
    background-size: 100% 50%; 
}

    The first number 100% is for the width and the second 50% is for the height.

    For the mobile view you need to do the following.

    body{
        background-size: 100% 100%; 
}

    you do not need to specify a position for the mobile view, since it should fill the whole body.

    Nice work on this challenge and Keep Coding.

    Marked as helpful
    1
  • HashimAlSadah•860
    @HashimAlSadahPosted over 2 years ago

    Hi @Blue-Cheesecake, congrats on completing this challenge. It looks really good, but I think there is problem in the overflow-hidden{ overflow: hidden}. It should be overflow-hidden{ overflow-x: hidden}, since you only want to hide the menu probably

    Great job and Keep Coding.

    Ignore this part. I cannot post the comment since it is too short, I had to add these lines here

    1
  • lazyCoder•50
    @AbbasvoraSubmitted over 2 years ago

    I have not done mobile design. I was facing difficulties designing the mobile version. I would love to have feedback on the same.

    Social Proof Section

    1
    HashimAlSadah•860
    @HashimAlSadahPosted over 2 years ago

    Hi @Abbasvora, great job implementing the desktop design. In fact, you have done the difficult one, which is the desktop. I think the mobile was difficult for you, because you started with the desktop first. The most common way and the easiest way is to do mobile first, then do the complex part, which is the desktop. As you can see in the mobile design all the containers are stacked above each other, straight forward, nothing is difficult.

    I recommend the course conquering responsive layouts by (Kevin Powell), which will help you out with responsiveness.

    Also, I think you forgot the background image in your page :).

    Anyway, nice job and Keep Coding.

    1
  • HashimAlSadah•860
    @HashimAlSadahPosted over 2 years ago

    Hi @whatTheFath, congrats on completing this challenge, your solution looks fine, but as you mentioned this is not the best JS practice.

    What you did in your code is that you gave each button an event listener, but the common method is to listen to any click event on the parent container, which is <div id="rate-btn" class="card-rate"> in your code. Then you can check if clicked element is a button. In you case each button has class of <button class="card-rate__list">1</button>.

    I have modified your code (JS part mainly), so you can learn how to listen to event more efficiently. I have also removed the <span> tag from the buttons(they were unnecessary). You can check the modifications and the comments on the following codePen.

    I also recommend the following tutorial by (Dev Ed) on how to create a to-do list, where you will learn how to listen to events and manipulate the DOMs.

    Overall, you have done a great job in this challenge and Keep Coding.

    Marked as helpful
    1
  • HashimAlSadah•860
    @HashimAlSadahPosted over 2 years ago

    Hi @yomidev, congrats on completing this challenge. I think that you are struggling with how to listen to events and how to manipulate the DOMs.

    Anyway, I have looked at your code and modified the javascript part and added comments, so you can understand it clearly and I also add an id to the submit button in html. the modification on codepen in the following link

    I also recommend that you watch the following tutorial by (Dev Ed) on how to make a to-do list web. It will help you to understand eventListeners and manipulating the DOMs

    Anyway, you have a good CSS skills and you have done an amazing work in styling the page, so Keep Coding.

    Marked as helpful
    0
  • Anna Lemonade🍋•290
    @pbitonga17Submitted over 2 years ago

    My 4th project for my week 3!

    I tried to code the mobile design first because according to some people in this community that it is a good practice to do mobile first before it goes to larger device. So.. I'm not sure if i did it correctly.

    Please feel free to comment if you see any minor/major issue! Thank you so much!! <3

    Responsive 3 column preview component

    1
    HashimAlSadah•860
    @HashimAlSadahPosted over 2 years ago

    Hi @pbitonga17, congrats on completing this challenge. Your code looks clean and the website is responsive, there is nothing to be criticized, you might want to add some margins to the main container or padding to the body to make sure that the edges of the page do not touch the main container when resizing the browser window.

    Also you have one accessibility issue, where you must have headings in your html code and you start form h1, h2, h3...etc. So, in you case you must have at least one <h1></h1>.

    Other than that, you have done an amazing job. keep Coding and keep doing mobile first :).

    0
  • HashimAlSadah•860
    @HashimAlSadahPosted over 2 years ago

    Hi @2yool, congrats on completing this challenge. Your text alignment seems fine, but you have some accessibility problems in your html code, which you can easily fixed by the following.

    -You should at least have one main tag <main></main> in your code. You could do that by replacing <div class="container-fluid"></div> by <main class="container-fluid"></main>.

    -You should also make the <div class="attribution"> as footer <footer class="attribution"></footer>

    Overall, great job and Keep Coding.

    Marked as helpful
    0
  • HashimAlSadah•860
    @HashimAlSadahPosted over 2 years ago

    Hi @saksham747, congrats on completing this challenge, but I have few tips that will improve your solution.

    First, to answer your question the best way to add the space between the container <div class="box"> and the image is to use padding for the box container, which will result in equal space in all directions.

    I also noticed that you are using a lot of fixed heights and widths in you code, which makes you struggle with responsiveness and make you add unnecessary margins and padding.

    I have looked at your code and commented the unnecessary lines and add others instead. it is in the following codepen, please take a look at it.

    I also recommend the free course about responsive layouts by (Kevin Powell).

    You can also look at his channel on YouTube in following link.

    Overall, great job and Keep Coding

    Marked as helpful
    0
  • Priyanshi Somani•100
    @Priyanshii677Submitted over 2 years ago

    Hello Everyone!! I am a beginner in Frontend world and I am practicing challenges regularly. I think I have improved very much. Please take a moment to review my solution and do share your feedback. Happy coding.

    Stats preview card component solution

    2
    HashimAlSadah•860
    @HashimAlSadahPosted over 2 years ago

    Hi @Priyanshii677, congrats on completing this challenge. Your code looks simple and easy to read which is great, but I have few tips that can up your game.

    1-Try to use (em) unit for padding and margin instead of pixels, so you can have easy and responsive layout.

    2- line-height does not have unit. it should be for example line-height: 1. where 1 refers to the space that one line occupies within the container.

    3-For the border radius, you do not have to specify a value for each corner, you could user one for the main container and if the edges overflow the container, you set overflow: hidden for the main container.

    4-To make sure that your main container does not grow infinity as the size of the window increases, set max-width to certain value. for example , max-width: 800px.

    I have looked at you code and made some changes and comments to it. you can find it in the following codePen.

    Also I recommend the free course (responsive layouts by Kevin Powell). I think it will help you a lot

    Anyway, you did good job with this challenge and Keep Coding.

    Marked as helpful
    1
  • HashimAlSadah•860
    @HashimAlSadahPosted over 2 years ago

    Nice work completing this challenge. You should at least have one <main></main> container in your html code. (probably, you should replace <section> with <main>). Also you should replace h2 with h1 since you have to start by order h1, h2, h3, etc... have the attribution in <footer></footer>. (I have done all the mistakes above before :) ) These steps should solve most of your accessibility issues. Overall, great work.

    Marked as helpful
    2