Skip to content
OverviewSolutions
16
Comments
20
Muh Suryadi Triputra Rahman
@msuryaditriputraR

All comments

  • Muh Suryadi Triputra Rahmanโ€ข400
    @msuryaditriputraRPosted almost 2 years ago

    Hi There ๐Ÿ‘‹

    Congrats on finishing this challenge ๐Ÿฅณ

    The place-items: center only works when the element is a grid, so just change the body's display to grid with display: grid, and don't forget to set the min-height of the body to 100vh;

    for your Accessibility report โš’

    • Wrap all of the content with <main>tag, All page content should be contained by landmarks.
    • If you just have one heading, you have to use h1 tag for semantic meaning and you can change its style but not the tag. The page should contain a level-one heading. change your h2 tag to h1

    I hope this is helpful๐Ÿ˜Š after you fix the accessibility issues, don't forget to generate a new report! ๐Ÿ“–

    keep it up your good work, Happy Coding โ˜•

    Marked as helpful
    1
  • Julieta Perezโ€ข10
    @juliennzSubmitted over 2 years ago

    Hi. I find it difficult to center the whole div in different screens or even different browsers. I'm unsure about the CSS code, the order of the properties, I know it's really important. It works but I know it could be better, any kind of feedback it's useful.

    QR code component challenge hub

    2
    Muh Suryadi Triputra Rahmanโ€ข400
    @msuryaditriputraRPosted over 2 years ago

    Hi Julieta Perez ๐Ÿ‘‹

    Firstly, I wanna congrats you on your success finish this challenge ๐ŸŽ‰

    However i have some feedbacks for u ๐ŸŒผ

    Accessibility ๐Ÿ‘€

    • Wrap all of your content with the main tag. All page content should be contained by landmarks.
    • For heading text use h1 instead of h3 if you just have one heading on your page. Page should contain a level-one heading.
    • Wrap attribution div with footer and place it at the bottom of your page for a better experience, or if you want to keep it at the top wrap it with header.

    Code & Design ๐Ÿ› 

    • If you want to place your div at the center of the page with position property. You should use transform: translateX(-50%) when you set the left with 50% and transform: translateY(-50%) when you set the top with 50%. or you just need to use transform: translate(-50%,-50%) for shorthand. learn more about translate
    • For best practice and easy way to make content centered is use grid layouting โšก
    main{
    display: grid;
    place-content: center;
    min-height: 100vh;
}

    and trada it's magic โ˜ƒ . learn more about grid layouting.

    I hope this helps, keep it up your good work and Happy Coding โ˜•

    Marked as helpful
    0
  • Muh Suryadi Triputra Rahmanโ€ข400
    @msuryaditriputraRPosted over 2 years ago

    Hi Icekid35 ๐Ÿ‘‹

    You have amazing work.. Good job ๐Ÿ™Œ then adding a copy feature and adorable icons are an impressive idea. Nice! ๐Ÿค˜

    However, i have some suggestions for u

    • You should wrap the advice title with h1 to fix your accessibility issue do you have. Page should contain a level-one heading
    • Add some padding in the copy element and add border-radius too.
    • For Divider, actually you can use the image assets but it's no big deal. Nice try to build your own divider style ๐Ÿ˜

    I see you have fixed your code according to the suggestion of the previous comment, but you forget to Generate a new report. Please update them ๐Ÿ› 

    I hope this helps. Keep it up your good work ๐Ÿ”ฅ๐Ÿ’ฅ

    Happy Coding โ˜•

    0
  • Muh Suryadi Triputra Rahmanโ€ข400
    @msuryaditriputraRPosted over 2 years ago

    Good Work, Nice Animations !

    1
  • Muh Suryadi Triputra Rahmanโ€ข400
    @msuryaditriputraRPosted over 2 years ago

    Hi Shane Pinder ๐Ÿ‘‹

    You have an interesting idea to make an image purple with a linear-gradient. But I think the best practice and easier is to use the filter property. check the explaination about the filter property. And you can also use css filter generator to make it easier ๐Ÿ˜„

    For the Accessibility issue do you have, you should wrap the attribution div with footer tag

    I hope this helps and happy coding โ˜•

    Marked as helpful
    0
  • Shashree Samuelโ€ข9,260
    @shashreesamuelSubmitted over 2 years ago

    Feel free to leave your appreciated feedback on my solution since I don't have any particular section of the challenge that was giving me trouble. Your feedback is very much appreciated.

    Thanks and happy coding ๐Ÿ‘

    Order Summary Component

    1
    Muh Suryadi Triputra Rahmanโ€ข400
    @msuryaditriputraRPosted over 2 years ago

    Hi TheCoderGuru ๐Ÿ‘‹ You have as good work as always ๐Ÿ’ช๐Ÿ”ฅ

    However, I think you didn't notice some detail from this challenge:

    • Background-color of the card content is white.
    • The one that should have background-color : very-pale-blue is the card-lower.
    • The icon doesn't have to have alternative text, just leave it blank.
    • The button should have cursor: pointer when it hovers.
    • You forgot to handle the button's state when it hovers.
    • IMHO on this challenge, since the anchor link doesn't point to any page, the best practice is to assign the href="#" attribute to it. so that when it is clicked the page does not reload.

    Actually we can't access your repo, I just look at your code through DOM element. so, i hope you can update it later.

    last but not least, i hope this feedback can help you ๐Ÿ™Œ

    Keep up your Good Work ๐Ÿ›  and Happy Coding โ˜•

    2
  • Suryadi Harunโ€ข10
    @suryadihrn08Submitted over 2 years ago

    Can I get any sugestion for this challenge ? thanks

    Using CSS

    2
    Muh Suryadi Triputra Rahmanโ€ข400
    @msuryaditriputraRPosted over 2 years ago

    Hi Suryadi , we have the same name ๐Ÿ˜…

    First I wanna congrats you for success to finish this challenge ๐Ÿ˜Ž๐ŸŽ‰ and welcome to the Front-End Mentor community ๐Ÿ”ฅโšก

    but I have some feedback for your solution โš’

    • Accessibility Issues
    1. Wrap all of the content with <main> tag, All page content should be contained by landmarks.
    2. If you just have one heading, you have to use h1 for semantic meaning and you can change its style but not the tag. The page should contain a level-one heading.
    3. For the div's attribution, you should wrap it with footer, for better accessibility.
    • Code & Design
    1. You should use CSS for change the background of body. Avoid using bgcolor attribute.
    2. You forget to change the color of the card title, Please check the style guide to see the code hsl colors.
    3. For a style that is close to the origin, you should use box-shadow on the card

    I hope this helps and can improve your code ๐Ÿ™Œ

    Happy Coding โ˜•

    Salam dari Makassar ๐Ÿ‘‹๐Ÿ˜

    Marked as helpful
    2
  • Muh Suryadi Triputra Rahmanโ€ข400
    @msuryaditriputraRPosted over 2 years ago

    Halo TheCoderGuru, Thanks for your helpful feedback..

    Cheers

    0
  • Muh Suryadi Triputra Rahmanโ€ข400
    @msuryaditriputraRPosted over 2 years ago

    Hi, I think you forget to add state for the image when it hover/active.. check the design assets for detail..

    and to avoid accessibility issue, wrap the attribution with footer

    0
  • Shane Pinderโ€ข190
    @ShanePinderDevSubmitted over 2 years ago

    Here's my solution using CSS Flexbox as well as absolute position to position the icon-view svg in the active state.

    Please let me have any suggestions you have for improving my code.

    NFT preview card component

    3
    Muh Suryadi Triputra Rahmanโ€ข400
    @msuryaditriputraRPosted over 2 years ago

    Hi There ๐Ÿ‘‹

    I wanna congrats you for success to finish this challenge ๐Ÿ˜Ž๐ŸŽ‰

    but I have some feedback for your codes โš’

    1. I think you don't need to use sections tag, use div instead.
    2. Use box-shadow for adding shadow on your card, not border.

    I hope this helps and can improve your code ๐Ÿ™Œ

    Happy Coding โ˜• Danbo

    Marked as helpful
    0
  • Muh Suryadi Triputra Rahmanโ€ข400
    @msuryaditriputraRPosted over 2 years ago

    I think you only need to put your CSS style on your stylesheet, not in inline style HTML tags

    for easy to readable, maintainable, and to be consistent

    Marked as helpful
    1
  • Muh Suryadi Triputra Rahmanโ€ข400
    @msuryaditriputraRPosted over 2 years ago

    For Scale something you can use transform: scale(x,y) for detail you can check this Link.

    Hopefully it is helpful..

    Marked as helpful
    0