Skip to content
OverviewSolutions
10
Comments
7
Martín
@martinorue

All comments

  • Martín•240
    @martinoruePosted over 1 year ago

    Hi! Great job! 😊

    Regarding your question, there is a better solution for these cases. The rule of thumb that I like to remember is that if the image contains important content for the user and is not just a background image, it should be placed in the HTML. Otherwise, it can be placed in the background CSS property.

    I would recommend you to read this article about responsive images. You'll find the anwser to your question in the Art Direction section.

    Good luck! If you need any further assistance, feel free to reach out.

    Marked as helpful
    1
  • Onyinyechi Viola Obika•130
    @ObikaviolaSubmitted almost 2 years ago
    What are you most proud of, and what would you do differently next time?

    I'm proud that I got to build a page using flex layout which was really hard for me to grasp. Next time, I will try to make it well aligned.

    What challenges did you encounter, and how did you overcome them?

    One of the difficulty I experienced was on the layout and I overcame it by learning and using flex layout.

    What specific areas of your project would you like help with?

    I'll need help with ideas on how to center the page vertically.

    Landing page on the frontend mentor QR code challenge

    3
    Martín•240
    @martinoruePosted almost 2 years ago

    Hi 👋

    For the .container it is recommended to use max-width insted of width so that by shrinking the width of the screen, the width of the container is flexible. Check out this

    In this case I think that you don't need to add height in .qr-code-image because when you use width in an <img> by default the height will be height: auto. And this is useful because you don't lose the aspect-ratio when you increase the image width.

    On the other hand, you added width: 100% to the <h1> but you don't need that because de <h1> element is display: block. And doing so can cause you problems, check out this. And maybe you wanted to use padding instead of margin to add space to this element.

    Hope this helps

    Marked as helpful
    1
  • Martín•240
    @martinoruePosted almost 2 years ago

    Hi 👋

    In this case you could try adding width to your <span> with the class "icon". But first you need to change the display property (which by default is "inline" in the <span>). So, in your <div> with the class "footer-icons" you could do something like this:

    .footer-icons {
    ...
    display: flex;
    justify-content: center;
    align-items: center;
}

    And then you can add width to your icon:

    .icon {
   ...
    width: 11px;
}

    I hope this helps you!

    1
  • Martín•240
    @martinoruePosted almost 2 years ago

    Hi, nice solution!

    When I uploaded my solution for this project, I got some fixes that might be useful to you too. Like the use of the <del> tag and image handling (here it would not be ideal to use background-image because the image is not just a background but is more relevant to the user).

    Take a look at the comments that @vcarames made me here

    Marked as helpful
    2
  • Martín•240
    @martinoruePosted almost 2 years ago

    Hi, great solution! 😊

    I noticed that even when I don't select any number, the form allows me to submit the feedback 😲

    I don't know if it's in your interest to focus on learning that now, but it could be interesting to get more out of this challenge 🤓

    Good luck!

    0
  • Martín•240
    @martinoruePosted almost 2 years ago

    Hi, excellent work!

    Maybe you forgot to add the button hover effect?

    And, in the case of the passed price, I think it would be nice to use the del element.

    1
  • Marco•20
    @MarcoPonce91Submitted almost 2 years ago

    Buenas noches! Es mi primer desafío en HTML y CSS, la verdad que lo fui codificando y visualizando en la medida que avanzaba con el código. Me costó mucho el poder adaptarlo al tamaño que se requería, a mi parecer y por ser la primera vez en esto, no lo veo tan mal. Quisiera consejos para mejorar y obtener buenas prácticas. Saludos desde Catamarca - Argentina.-

    QR Code Component

    1
    Martín•240
    @martinoruePosted almost 2 years ago

    Buenas! Lo veo bárbaro.

    Yo usé flexbox para centrarlo en el body. Si todavía no arrancaste con eso, te recomiendo este recurso para aprenderlo.

    Por otro lado, el warning que te está tirando el reporte de accesibilidad tiene que ver con el uso de un div en lugar de un elemento semántico o, en su defecto, un landmark.

    Suerte!

    Marked as helpful
    0