Skip to content
OverviewSolutions
6
Comments
152
Fatlind Shehu
@fatlindshehu

All comments

  • Fatlind Shehu•2,230
    @fatlindshehuPosted over 2 years ago

    Hi, Nice work with the challenge, although I would recommend centering the component using flexbox and not margins, here's what you can do:

    • display: flex to set the div/component as a flex container.
    • justify-content: center to center the div/component horizontally.
    • align-items: center to center the div/component vertically.
    • Make sure the div/component has a height.
    • If you’re unfamiliar with flexbox, I would suggest checking THIS

    Also, I would recommend avoiding the use of px together with rem, instead, go only with rem

    Keep it coding...

    Marked as helpful
    0
  • Trenton Bensheimer•190
    @tbensheimerSubmitted over 2 years ago

    Hello everyone! I completed another project and this one was one of the harder newbie challenges in my opinion but I still enjoyed doing the project! I had fun figuring out how to lay the image perfectly next to the div container. I also never knew that you can use the border-radius property on certain corners which was interesting. If any of you have any suggestions on how to improve my code please let me know! Thank you in advance!

    Product Preview Card

    1
    Fatlind Shehu•2,230
    @fatlindshehuPosted over 2 years ago

    Hi @tbensheimer

    There is a way of seting the border-radius only on the container and showing also on the image, you can do that like this:

    .container{
  border-radius: 10px;
  overflow: hidden;
}
    • I would suggest using rem or em units, instead of pixel units, px can cause Issues when trying to make your site/component responsive.

    Now you can remove the border-radius on the image!

    Keep it up :)

    0
  • Fatlind Shehu•2,230
    @fatlindshehuPosted over 2 years ago

    Hi,

    Your component looks good, but you have a hover state you need to work on, in case you don't know how to approach that, here's my suggestion:

    • Create a div and place it above the image, using position: absolute
    • Center the div:
    display: flex to **set** the *div/component* as a *flex container*.
justify-content: center to **center** the *div/component* horizontally.
align-items: center to **center** the *div/component* vertically.
    • Set div opacity: 0 to hide the div on normal state.
    • Set div:hover opacity: 1 to show the div on the hover state, make sure you add the background based on the instructions given by Frontend Mentor

    Keep up the good work!

    Marked as helpful
    0
  • Fatlind Shehu•2,230
    @fatlindshehuPosted over 2 years ago

    Hi @candybuy

    Nice work with the component, I think you need to be more carefull with the spaces between elements!

    • I would suggest measuring heights/widths & paddings/margins with more precision, using a tool like FIGMA or Adobe XD helps you a ton!
    • Try not to use two images for two different screen sizes, instead make the image responsive by setting its width/height with %.

    Keep up the good work :)

    0
  • Fatlind Shehu•2,230
    @fatlindshehuPosted over 2 years ago

    Hi @panosjapan7

    Your component looks great, I would recommend some improvements:

    • Adding a padding to main in order to avoid the overflow from the browser, make sure to adapt the padding corresponding to the device's screen size.
    • Using a pre-processor like SASS or LESS would be also very nice, for bigger projects, pre-processors make the code reusability and maintenance very easy and also reading your code from other developers is much easier.
    • I would suggest using rem or em units, instead of pixel units, px can cause Issues when trying to make your site/component responsive.

    Keep up the good work!

    Marked as helpful
    1
  • Fatlind Shehu•2,230
    @fatlindshehuPosted over 2 years ago

    Hi @ADinezio

    Your component looks great,

    • I would suggest adding margin to the component, in order to prevent the browser from overflowing the component (This happens when resizing from bigger screens to medium ones)
    • You can also set a border radius for the component, (not individually for each card) and use overflow: hidden to make it appear responsively.
    • Using a CSS naming convention would be a nice addition to your skills as a Frontend Developer, my suggestion is to use BEM which stands for Body Element Modifier, check this LINK if you’re interested in knowing more about BEM

    Keep up the good work!

    Marked as helpful
    0
  • Trey•20
    @treywmohammedSubmitted over 2 years ago

    Looking for feedback. First real project besides tutorials.

    QR Scanner

    1
    Fatlind Shehu•2,230
    @fatlindshehuPosted over 2 years ago

    Hi @treywmohammed

    Congrats on your project, I would suggest checking the spaces between elements more carefully, I would suggest measuring heights/widths & paddings/margins with more precision, using a tool like

    • FIGMA or Adobe XD helps you a ton!

    I would also suggest centering a div/component using flexbox: -display: flex to set the div/component as a flex container.

    • justify-content: center to center the div/component horizontally.
    • align-items: center to center the div/component vertically.
    • Make sure the div/component has a height.
    • If you’re unfamiliar with flexbox, I would suggest checking THIS

    Check this out & let me know how it goes!

    Happy coding...

    0
  • Fatlind Shehu•2,230
    @fatlindshehuPosted over 2 years ago

    Hi @Isaque803

    Centering the component using absolute positioning probably is not the best choice I would recommend: Centering a div/component using flexbox: -display: flex to set the div/component as a flex container. -justify-content: center to center the div/component horizontally. -align-items: center to center the div/component vertically. -Make sure the div/component has a height. -If you’re unfamiliar with flexbox, I would suggest checking THIS

    Also is recommended using rem or em units, instead of pixel units, px can cause Issues when trying to make your site/component responsive.

    Either way nice work & keep it going...

    Marked as helpful
    0
  • Fatlind Shehu•2,230
    @fatlindshehuPosted over 2 years ago

    Hi

    Your component looks pixel-perfect, I have nothing to add about that, I have some general suggestions for you:

    • I would suggest using rem or em units, instead of pixel units, px can cause Issues when trying to make your site/component responsive.
    • Using a CSS naming convention would be a nice addition to your skills as a Frontend Developer, my suggestion is to use BEM which stands for Body Element Modifier, check this LINK if you’re interested in knowing more about BEM

    Keep up the good work!

    Marked as helpful
    1
  • Fatlind Shehu•2,230
    @fatlindshehuPosted over 2 years ago

    Centering a div/component using flexbox:

    • display: flex to set the div/component as a flex container.
    • align-items: center to center the div/component vertically.
    • Make sure the div/component has a height in order to center it vertically.
    • justify-content: center to center the div/component horizontally.
    • If you’re unfamiliar with flexbox, I would suggest checking THIS

    Keep up the good work

    Marked as helpful
    1
  • Fatlind Shehu•2,230
    @fatlindshehuPosted over 2 years ago

    Hi there,

    The component looks pretty good, I would suggest some general coding skills for future projects:

    • Try using only one unit, I would suggest using rem and avoid the use of pixel units
    • A CSS naming convention would be a great addition to your skills, I would recommend BEM which stands for Block Element Modifier.
    • Also the use of a CSS Pre-processor would be nice, on bigger projects it helps maintaining the code and making it reusable!

    Keep up the good work…

    Marked as helpful
    0
  • Prathama-Redij•30
    @Prathama-RedijSubmitted over 2 years ago

    It was little bit easy task for me. I really enjoyed coding while doing the task.

    QR Code

    1
    Fatlind Shehu•2,230
    @fatlindshehuPosted over 2 years ago

    Hi @Prathama-Redij,

    This challenge seems easy but sometimes people struggle with it, I can see you did nice work so congrats on that!

    • I can see you centered the component using flex
    • I think this code
    .container {
    max-width: 360px;
    /* margin-top: 10%; */
    /* margin: 0 auto; */
    /* display: flex; */
}

    on the .container is unnecessary ( The commented one)

    • I would suggest using rem or em units, instead of pixel units, px can cause Issues when trying to make your site/component responsive & avoid mixing them (using more than one)

    Keep up the good work!

    Marked as helpful
    0