Skip to content
OverviewSolutions
5
Comments
5
Sebastian
@Abestian

All comments

  • Sebastian•90
    @AbestianPosted about 10 hours ago

    Hello @Afong, I really like your solution, great job!

    I've tested it on live server, glanced through your code and this is what I came up with:

    • The card is an exact match to the design, I love it
    • Hover effect is great, but I would recommend adding a transition for the background color (e.g. background-color 0.3s) to make the change smoother
    • Card is responsive, you could change the breakpoint for desktop view though. At current 1440px I think it's just a little too far
    • You used rems for some properties, and it's great! You should also include ems in the future, as rems is typically recommended for font-sizes and ems for the rest of properties. A quick video from Kevin Powell should explain it well CSS em and rem explained
    • There's no cursor: pointer property applied to the button, which can be confusing for users
    • You should avoid setting card sizes (height, width) in pixels, doing this can break responsiveness of your card, it's better to use percantages or vw (or to leave it all and let the content or grid/flexbox dictate it's size)
    • It's not visible here, on frontend mentor slider, but when I opened live site I had seen a white line under the image on the desktop version. To fix it, you could add height: 100% property to the desktop media query to ensure that the image fills its container properly

    That's all from me, I hope my review will help you (and also I hope it doesn't look too harsh!)

    0
  • Kellen James•120
    @KellenkjamesSubmitted 12 days ago
    What are you most proud of, and what would you do differently next time?

    I'm most proud of taking extra time to build a design system which is modular and scalable (without needing to leverage heavier CSS frameworks). In addition, I'm proud of the end result which looks clean and professional.

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

    Layout and structure was a little bit of a challenge since this page contains a lot of content. In addition, building the table was a little bit tricky as well as handling some of the position and spacing of elements on mobile.

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

    N/A

    A modern, recipe card component, built using Angular and SASS

    1
    Sebastian•90
    @AbestianPosted 5 days ago

    I love it, your solution greatly resembles the initial design, layout looks good on every possible display screen. Great job!

    1
  • Sebastian•90
    @AbestianPosted 8 days ago

    Great Job on Your Solution!

    First off, I just want to say you did an amazing job with your solution! It's almost identical to the provided design and the hover effect on clickable elements is spot-on.

    Since I haven't worked with React yet, I won’t dive into the code. I have some suggestions on the card itself though, and I think they could make your solution even better:

    1. Add Margin to Card Sides

    When the screen width goes below 355px, the card starts to stick to the sides of the screen. A small margin on the sides would fix this and make the design look cleaner on smaller screens.

    2. Smooth Hover Transition on Links

    The hover effect on the links is a bit too sudden. Adding a transition delay could smooth it out and give it a more polished feel.

    3. Minor Cosmetic Tweaks

    These are small details that won't break the functionality but could improve the overall experience:

    3.1 Fix Content Shifting with Different Screen Heights

    When you stretch the screen vertically, the card content shifts because of justify-content: space-around. It would be better if the content stayed in place regardless of the screen height.

    3.2 Text Alignment for Small Screens

    When the screen width gets below 200px, the text in the upper part of the card starts to stick to the sides. A quick fix here is to add text-align: center; to ensure the text stays centered

    These are just a few thoughts to help make your already great solution even better! Keep up the fantastic work!

    0
  • Sebastian•90
    @AbestianPosted 13 days ago

    Hey! Great job overall! The card and the code look solid, and you've made a really great job at your solution. There are just a few things to refine, which could elevate the project even further.

    What’s working well:

    The code is clean and easy to read. The solution is very close to the design, which shows you’ve understood the requirements well.

    Suggestions for improvement:

    1. When using <h$> tags, try to use them in a cascading order from <h1> to <h6>. Currently, the project only uses <h3>, but keeping that hierarchy intact can help with SEO and accessibility.
    2. It would be a good idea to check the responsiveness across different screen sizes before finalizing. For example, the card looks great between 950px and 1250px, but it could use some adjustments for other screen widths.
    3. The title could benefit from being wrapped in an <h> tag for semantic HTML. Additionally, adding a hover effect to the title would improve the user experience, and I think it should be treated as a link in this project.
    4. Don't forget to add values to "alt" attributes inside your <img> tags. It’s essential for accessibility and SEO.

    Again, you’re doing really well, and these tweaks will only make your project even stronger. Keep up the good work!

    Marked as helpful
    0
  • Sebastian•90
    @AbestianPosted 15 days ago

    The solution incorporates semantic HTML, ensuring accessibility and a responsive layout that adapts well across most screen sizes. However, at a width of 320px (like on an iPhone 5/SE), the card feels cramped and could benefit from additional padding or margin. Overall, the code is well-structured, and the implementation closely mirrors the original design.

    Marked as helpful
    0