Skip to content
OverviewSolutions
9
Comments
9
P
Josh Boys
@jboys

All comments

  • Josh Boys•180
    @jboysPosted 22 days ago

    Really nice solution, mofada!

    Your solution's responsive design is a very close match to the target design. The only suggestion I have here is to constrain the width of the card on the mobile layout (i.e. maybe set a max width to something like 500px for this stacked layout).

    Your panel and menu work well. I would allow the menu to be closed by clicking on the share icon again, otherwise the user might think it is stuck open.

    I like how you dismiss the panel/menu by clicking outside of them — nice touch! Though, this only works within the card.

    Overall, great job!

    0
  • Josh Boys•180
    @jboysPosted about 1 month ago

    Great solution Ehsan! It matches the design closely and the responsiveness is on point. Code is neat. Good use of components. Well done.

    Only a few minor improvements/suggestions:

    • Perhaps make further use of semantic tags (like: section, header, nav)
    • Alt text could have been more descriptive in places
    • Consider simplifying component variations (e.g. buttons with different colours) by accepting props for these variants

    But these are rather minor things.

    Marked as helpful
    1
  • Kaczupinko•170
    @KaczupinkoSubmitted 3 months ago
    What are you most proud of, and what would you do differently next time?

    I will put more effort on responsiveness.

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

    I had a lot of trouble transitioning from mobile to tablet responsiveness and i think is better to gain some knowledge about it.

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

    Any feedback is welcome.

    Testimonials-grid-section HTML TAILWIND

    1
    Josh Boys•180
    @jboysPosted 2 months ago

    Hey Kaczupinko,

    Nice job with your solution! I love how well it works on both mobile and desktop.

    One thing I noticed though: on screens less than 1280px (Tailwind’s xl breakpoint), it might look even better if the cards didn’t stretch across the full width. Maybe you could try using something like max-w-80 w-full to constrain them a bit.

    Also, adding one or two intermediary layouts (like grids with 2 or 3 columns) between mobile and desktop would give it a smoother transition across different screen sizes, but it’s not required for this challenge.

    Great work overall 👍

    Marked as helpful
    0
  • Josh Boys•180
    @jboysPosted 2 months ago

    Hey Rahmonbek,

    I really like your solution! It looks clean, responsive and closely matches the design — but I noticed you've written a lot of custom CSS in your input.css file. Since you're using Tailwind, you could simplify things by relying more on its utility classes.

    For example, instead of writing custom margins, paddings, or flex layouts, you can use Tailwind's built-in utilities like p-6, mt-4, md:flex-row, and shadow-lg. Also, Tailwind can handle screen size breakpoints with prefixes sm: md: etc, which saves you from writing custom media queries. Making better use of Tailwind will reduce the amount of CSS you write and should make future adjustments easier.

    1
  • Josh Boys•180
    @jboysPosted 2 months ago

    Hey Rahmonbek,

    I've gone through your code and viewed the product preview card. Your layout for desktop looks really good. Here's a few things that could improve your solution:

    Responsiveness:

    • The card doesn't adapt to smaller screens and causes horizontal scrolling. The fixed width on the container (w-[640px]) is the main issue.
    • For the image and content layout, you can switch to flex-col on mobile screens and keep flex-row on larger screens using sm:flex-row.

    Semantics and accessibility:

    • The heading structure could be improved for better hierarchy. Using <h1>, <h3>, and <h5> feels inconsistent.
    • The alt text for the images should be more descriptive. Instead of "image-product-desktop", try something like "Gabrielle Essence Eau De Parfum bottle from Chanel".
    • Adding more focus styles and features like aria-label would improve accessibilty.

    Overall, your layout is good and codebase is clean. The main area to focus on is responsiveness, so the card looks good on all screen sizes.

    Cheers!

    1
  • Sipan Ahmad•160
    @sipanahmadSubmitted 8 months ago
    What are you most proud of, and what would you do differently next time?

    I'm proud of work with this framework and did it so good.

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

    The challenge i see it in this project is how to make the image in mobile screen.

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

    I really want to give this advice for every one solve this challenge watch the other project and improve you code also.

    Recipe page main using tailwindcss

    1
    Josh Boys•180
    @jboysPosted 2 months ago

    Hey Sipan,

    I had a look through your code, and it looks good overall! Just a few thoughts on how you could refine it a bit:

    Design

    • The Young Serif font isn't displayed, despite it being defined in your code
    • The top margin is far too large and pushing your content below the fold on Desktop; Mobile shouldn't have any margin.

    HTML5 Semantics and accessibility

    • Good use of main, footer, and headings (hierarchy)
    • <article> should wrap the whole recipe and each recipe section should be wrapped in a <section> rather than an <article>
    • Perhaps use a more descriptive alt text for the omelette image. Something like "Image of a cooked omelette served on a plate" would be more helpful for users with screen readers / limited vision.

    Code and best practices

    • I noticed you're using tabs for indentation. It's consistent and well-structured, but consider using spaces: it's easier to read — have a look how GitHub renders it. The extra space makes it harder to scan.
    • It would be cleaner to move your Tailwind setup/config out of the HTML.
    • You’re adding role="article" and role="main" in a few spots, which is unnecessary since those tags already have those roles by default.

    All in all, minor things. Good job!

    0
  • KorriganMaster•70
    @KorriganMasterSubmitted 2 months ago
    What are you most proud of, and what would you do differently next time?

    I've decided to adapt the mobile design for small screens to make the card full screen instead of centered with margins.

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

    The main tricky part for me was to include design required properties (font family and colors) into Tailwind CSS using custom config.

    I explored the Tailwind documentation to find out how custom theme works.

    Social links profile (Astro/Tailwind CSS)

    1
    Josh Boys•180
    @jboysPosted 2 months ago

    Great solution — I love the choice in responsiveness! Personally, I would mute the "Front-end developer and avid reader" text a little by making it off-white so it doesn't compete so much with the name and location.

    Can't comment on the code as I've only had a cursory look at Astro before. But it looks neat, both your solution (design) and codebase I browsed. Semantics and accessibility look on point.

    Marked as helpful
    1
  • Jared Peters•120
    @jrddpSubmitted 3 months ago
    What specific areas of your project would you like help with?
    • Are there ways I should be improving accessibility?
    • Semantically, what is the best tag to use for something like "Greg Hooper" here or "Published 21 Dec 2023"? Should it just be a non-descriptive span?

    Straight-forward TailwindCSS + HTML Solution

    1
    Josh Boys•180
    @jboysPosted 2 months ago

    Hi Jared,

    Nice solution! It looks super close to the target design and your code looks clean and was easy to read.

    A couple of sizes were slightly off (nitpicking 🙈):

    • The card's drop shadow should be 8px rather than 5px (shadow-[8px_8px_0_0])
    • The avatar should be 32x32px (size-8 rather than size-6)

    Also, in your tailwind.config.js, I'm not sure why you defined the base font size to be 16px. I thought this was the default by Tailwind?

    To improve accessibility, add an alt tag to the author's image (e.g. alt="Author Greg Hooper") and consider a more descriptive alt tag for the article image (e.g. alt="Illustration representing HTML and CSS concepts").

    To improve semantics, perhaps wrap the category tag and publication date in a header and the author details in a footer tag. The date should be in a time tag (Published <time datetime="2023-12-21">21 Dec 2023</time>).

    Also, I would extract the card into its own div rather than main to make it more reusable, as it is unlikely the card would be the only content on a page.

    Marked as helpful
    1
  • Josh Boys•180
    @jboysPosted 2 months ago

    The solution differs quite a bit from the design: there is no QR code in the "solution vs design" view (though in your repo, it is defined and looks correct) and the text needs adjusting to better match the design — font family, line height and the spacing between the heading and text below it.

    0