Skip to content
OverviewSolutions
22
Comments
14
Daniel Muri
@dmuri

All comments

  • Mateusz•180
    @matsta92Submitted about 1 year ago

    Hello, please rate the project - Product preview card component.

    I am a beginner.

    I will be happy to receive tips on any mistakes I may have made in the code. So as not to develop bad habits.

    Product preview card component

    1
    Daniel Muri•440
    @dmuriPosted about 1 year ago

    Hi there 👋

    Really nice work, for the image swapping it is considered best practise to use picture and srcset tags. See an example below

        <picture>
        <source media="(min-width: 800px)" srcset="large-image.jpg">
        <img src="small-image.jpg" alt="Description of image">
    </picture>

    Here the "large.img will be used for screens wider than 800px and small will be displayed on screens under that.

    Here is an article on the topic: https://developer.mozilla.org/en-US/docs/Learn/HTML/Multimedia_and_embedding/Responsive_images

    Keep up the good work my friend!

    Happy coding!

    0
  • Daniel Muri•440
    @dmuriPosted about 1 year ago

    Hi there 👋

    Well done, really nice work! A small suggestion would be adding a max-width property inside the media query as the main content gets quite streched. I would also consider changing the break point to around 750px, but that is of course a matter of taste.

    Happy coding!

    0
  • Daniel Muri•440
    @dmuriPosted about 1 year ago

    Hi there, well done!

    It looks like you are trying to center the whole component. Adding a min-height:100vh to the body, is the only thing missing for that.

    body { 
   min-height: 100vh;
}

    Hope that is helpful :)

    Happy coding!

    Marked as helpful
    1
  • Daniel Muri•440
    @dmuriPosted about 1 year ago

    Hi there 👋

    Well done! Looks like the github-link is to an empty repository.

    Happy coding!

    1
  • ronnypdev•70
    @ronnypdevSubmitted about 1 year ago

    This was a good one to start but I wasn't sure about the responsive part any feedback is welcome!

    QR Component Build

    1
    Daniel Muri•440
    @dmuriPosted about 1 year ago

    Hi there! 👋

    Well done! A small suggestion would be to add the background-color to the body element to avoid the grey background cutting off on wider screens.

    Happy coding!

    Marked as helpful
    0
  • Daniel Muri•440
    @dmuriPosted about 1 year ago

    Hi there, well done!

    It looks like site has some issues with the font. You can easly add this to your site by two small changes in the html and css.

    HTML, in the header tag:

    <link rel="preconnect" href="https://fonts.gstatic.com" crossorigin>
<link href="https://fonts.googleapis.com/css2?family=Outfit:wght@400;700&display=swap" rel="stylesheet">

    CSS:

    body {
   font-family: 'Outfit', sans-serif;
}

    I would recommend watching a video on adding fonts with "Google Font" to a project. https://www.youtube.com/watch?v=eLY5KSXcQ6c&ab_channel=RosemaryBarker

    This one is a short nice introduction to it.

    Happy coding!

    Marked as helpful
    0
  • Firerenza•60
    @FirerenzaSubmitted about 1 year ago

    Had some trouble getting the footer to stay at the bottom of the page, as well as getting the body to fill the entire page so that the main container could sit in the middle of the page. Not sure if everything is organized correctly, but it seems to work. Also I'm not sure if the "mobile" layout is responsive. I did not do anything special for that, and I'm not sure if I was supposed to.

    QR code component using html, css, and flexbox

    2
    Daniel Muri•440
    @dmuriPosted about 1 year ago

    Hi there, well done!

    It looks like site has some issues with the font. You can easly add this to your site by two small changes in the html and css.

    HTML:

    <link rel="preconnect" href="https://fonts.googleapis.com">

... Add these two lines
<link rel="preconnect" href="https://fonts.gstatic.com" crossorigin>
<link href="https://fonts.googleapis.com/css2?family=Outfit:wght@400;700&display=swap" rel="stylesheet">
...

<link rel="icon" type="image/png" sizes="32x32" href="./images/favicon-32x32.png">

    CSS:

    body {
   font-family: 'Outfit', sans-serif;
}

    I would recommend watching a video on adding fonts with "Google Font" to a project. https://www.youtube.com/watch?v=eLY5KSXcQ6c&ab_channel=RosemaryBarker

    This one is a short nice introduction to it.

    Happy coding!

    Marked as helpful
    0
  • Daniel Muri•440
    @dmuriPosted about 1 year ago

    Well done!

    I have some small suggestions to improve responsiveness for smaller displays.

    1. Remove height and width property from .main and let flexbox do the layout for you
    2. Add a media query for .main with flex-direction

    The CSS would look something like this

    .main {
    display: flex;
    flex-direction: row;
    box-shadow: 10px 10px 30px #342eea31;
    border-radius: 25px;
    background-color: white;
}

@media (max-width: 800px) {
    .main {
        flex-direction: column;
    }
}

    Happy coding!

    Marked as helpful
    0
  • Daniel Muri•440
    @dmuriPosted about 1 year ago

    Hi there,

    Well done, small suggestion is to add this adjustment to the .cart-money p styling:

    .cart-money p {
    text-decoration: line-through;
    color: grey; /* or the appropriate color for the design */
}

    Happy coding!

    Marked as helpful
    0
  • Daniel Muri•440
    @dmuriPosted about 1 year ago

    Hi there,

    Great job on the challenge! Just a couple of suggestions to enhance the background behavior:

    Consider adding background-position: top center; – this might help stabilize the background when adjusting screen sizes.

    Adding background-repeat: repeat-x; to prevent the background from cutting off at the edges on larger displays.

    Keep up the good work!

    0
  • Daniel Muri•440
    @dmuriPosted over 1 year ago

    I am not sure why the hover effect on the buttons for the two side plans are not comming through. If someone with react/tailwind experience could look at it I would greatly appreciate it! The hover effect is in PlanGroup.tsx line 68

    0
  • Gabriel Ferreira•150
    @WillamiFerreiraSubmitted over 1 year ago

    Hello, everyone ✌️

    This challenge take me much more time than I expected initially, all because of the image that is, for me, pretty complicate to work with yet. I really think there are much more to improve in that code, particularly on the Javascript.

    Any Feedback Is Greatly Appreciated!

    Huddle | SASS + JS

    1
    Daniel Muri•440
    @dmuriPosted over 1 year ago

    Hello, nice work!

    Actually the javascript is not needed in this project. There is a css pseudo class called active that has the behavior you are looking for. There are two very simple changes in the style.scss file you can make that will allow you to remove the script.js all together.

    .on{
    color: white;
    background-color: $softMagenta;
    box-shadow: none;
}

    Change .on to button:activate

    .social-actived{
    border: 1px solid $softMagenta;
}

    Change .social-activated to .s:activate

    Happy coding!

    0