Skip to content
OverviewSolutions
10
Comments
7
Tiago Costa Rebelo
@tiagocostarebelo

All comments

  • ownedbyanonymous•190
    @ownedbyanonymousSubmitted 4 months ago
    What are you most proud of, and what would you do differently next time?

    I am happy that l was able to complete the challenge and flow the mobile-first workflow. Befor going through the articles on Frontend Mentor l never understood why it was important to build for mobile first.

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

    I had difficulties trying to use different images for different device size. I feel my desktop is so off from what was given in the design. Creating for mobile was easy but optimizing the code to work for desktop or bigger screens was difficult

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

    Using assets optimized for different device sizes Still having knowledge gaps in the use of em and rem, Flexbox and media queries.

    Responsive product preview card component

    1
    Tiago Costa Rebelo•180
    @tiagocostarebeloPosted 4 months ago

    Hello @ownedbyanonymous,

    First, well done on submitting and pushing yourself to learn and improve.

    A few things I've noticed in your code:

    • Replace <div> with a <button> where a button is intended. This improves both semantics and accessibility. Also, ensure all interactive elements are keyboard-accessible and screen reader-friendly.

    • Your <picture> element contains two sources and one img. Check the documentation on the MDN. You only need one.

    • For a faster workflow, explore CSS custom properties and how to set/use them. This will streamline your workflow and make your code cleaner.

    • You are setting the width of your article (which seems to be your card container) to 40vw, which I believe is causing issues with the desktop version. A simpler approach would be setting the width to 100% and then a max-width to the desired value, like 600px or 700px. This will improve the scalability and responsiveness of the card. The same applies to height values.

    • I also advise against adding width to <p> elements. The parent containers should handle that, not the element itself.

    Hope this helps. Keep up the good work!

    Best regards, Tiago

    0
  • Tiago Costa Rebelo•180
    @tiagocostarebeloPosted 4 months ago

    Hello @Mercy1024,

    Good job on your submission.

    Here are a few observations and suggestions:

    Recipe Card Width: The width of the recipe card is not as wide as the design, causing the text to wrap incorrectly and giving it less space. Try increasing the card's width to match the design more closely.

    Preparation Time Section: The preparation time section has more spacing between its content than specified in the design. Reducing this spacing will help align it more accurately with the design.

    Flexbox Usage: You are using display: flex more than necessary. Simplify your layout by removing flex properties where they are not needed. This will make your code cleaner and easier to maintain.

    Keep up the great work!

    0
  • Mauricio•120
    @LucknaghSubmitted 4 months ago
    What specific areas of your project would you like help with?

    I had difficulty centering the text of the links, if you could give me feedback on other ways that I could have aligned I would appreciate it

    Social links profile

    3
    Tiago Costa Rebelo•180
    @tiagocostarebeloPosted 4 months ago

    @Lucknagh, Good job on your submission. It closely matches the challenge requirements. Well done!

    Regarding your question about centering the links, you can use the following approach:

    Apply text-align: center to the links parent container. Use width: 100% and display: block on all links to make them clickable and extend the full width of their parent container, giving them a button-like appearance and allowing them to be clickable even when not hovering the link text.

    Additionally, I'd advise against using fixed width and height for elements, as this can make responsiveness more challenging. Instead, set the width of the card container to 100% and use max-width or min-width to control its growth and ensure it adapts well to different screen sizes.

    Keep up the great work!

    Marked as helpful
    0
  • Mohamed-Oday•190
    @Mohamed-OdaySubmitted 4 months ago
    What are you most proud of, and what would you do differently next time?

    Learned how to center a form, ill try to be more creative next time

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

    Centering the form

    QR code component

    1
    Tiago Costa Rebelo•180
    @tiagocostarebeloPosted 4 months ago

    Hello @Mohamed-Oday,

    Great job on your submission!

    I have a few suggestions that might help you improve your project:

    • The background color doesn't match the original design. Adjusting this will bring your project closer to the intended look;

    • Try reducing the width of the card slightly. This will help the text wrap correctly and match the design more closely;

    • Understanding Semantic HTML is important, especially for accessibility. For instance, screen readers rely on proper HTML semantics to provide accurate information to visually impaired users. In your code, you used a form element to wrap the card, but there are no input fields or submit buttons inside. This can cause screen readers to misinform users. I recommend reading this link(https://developer.mozilla.org/en-US/docs/Glossary/Semantics) to learn more about why semantic HTML matters.

    • Improve your class naming conventions. This will help you in the future as your projects become more complex. Here's a useful link(https://getbem.com/naming/).

    Keep up the good work!

    Marked as helpful
    0
  • Tiago Costa Rebelo•180
    @tiagocostarebeloPosted almost 3 years ago

    Hello, You are on the right path and it will only get better.

    In your code, you need to close the <head> tag before starting the <body> tag. Also, assign classes to your HTML tags. You'll see how it helps your styling. Also, don't use <br> to create margin between HTML elements. Style them with margin-top or margin-bottom, depending on what you need. The same can be applied to the margins and how you set them in your paragraph. Check this link (https://www.w3schools.com/css/css_margin.asp)

    There's also a the uppercase titles and the borders on the left and right that are rounded and the button, it doesn't take the full width.

    And for accessibility issues to be removed, put everything inside a <main> tag.

    Keep it up!

    Marked as helpful
    0
  • Tiago Costa Rebelo•180
    @tiagocostarebeloPosted almost 3 years ago

    Ah awesome job. It's really nice to use the bar to preview your solution vs design and see that it matches. Great work!

    1
  • Tiago Costa Rebelo•180
    @tiagocostarebeloPosted almost 3 years ago

    Hello, Overall good job, except the text on the desktop version has the opposite margins top and bottom. Notice that the margin is bigger on the bottom than on the top, opposite to the design. And give your title a bit more of size.

    Keep it up!

    Marked as helpful
    0