@sumaiyakawsar
What specific areas of your project would you like help with?
I'm open to feedback on improving my code efficiency, structure, and best practices. Let me know if you spot any areas for optimization or better approaches!
P
BlonoBuccellati
@BlonoBuccellatiAll comments
- P@BlonoBuccellati
great job!
- @Mriganka5137P@BlonoBuccellati
great job!
- @Fikerte-TP@BlonoBuccellati
I’d like to share a few thoughts.
- When the
<input>receives focus, the UI’s height increases—likely because a border is being added on focus. To prevent that, use an outline instead to avoid this layout shift. An outline is not included in the element’s box size. - The reset button isn’t being disabled based on conditions; consider adding the disabled attribute to the
<button>to prevent it from being clickable when it shouldn’t be.
Hope this helps!
- When the
- P@mehmetcagriekiciP@BlonoBuccellati
nice
- @MoriNo23What are you most proud of, and what would you do differently next time?
- Proper Use of min-height in Layouts
"How can I use min-height effectively to prevent layout issues? I learned it’s useful for ensuring a container doesn’t shrink too much, but I’m unsure how to balance it with other properties (like height or flex-grow) to avoid unexpected expansion."
Key points:
What challenges did you encounter, and how did you overcome them?You recognize its importance for responsive containers. You want to avoid conflicts with other layout rules. You’re seeking practical examples (e.g., min-height in Flexbox/Grid).Optional: Code Structure Feedback
"Are there ways to simplify my form validation logic or CSS structure? I’d appreciate feedback on making my code more maintainable."
Key points:
Open-ended request for optimization. Targets readability/scalability.P@BlonoBuccellatiI’d like to share a few thoughts.
About
heightandmin-heightI generally avoid using
heightin layouts, and instead rely onmax-width, padding, and flow-based layouts. Setting a fixedheightcan easily cause content to overflow or become clipped, especially on smaller screen sizes.As you mentioned,
min-heightis useful when you want to ensure a container keeps a minimum size — such as for modals. In this specific UI, I would usemin-heightfor the modal buttons on mobile view, to ensure they are always placed near the bottom of the viewport. For that, I usemin-h-screenin Tailwind CSS, which is equivalent tomin-height: 100vh.
About the structure
It seems the GitHub repository only includes the compiled source code, so unfortunately I wasn’t able to review the layout structure itself.
On form validation logic
I recommend using custom hooks to isolate form logic, such as validation, input state, and submission handling. This improves readability and reusability — especially as your forms become more complex.
On simplifying CSS
There are a few key principles I follow to keep CSS maintainable:
-
Mobile-first design: Start by designing for the smallest screen first. It often leads to simpler layout logic. I recommend this article:
How to Take the Right Approach to Responsive Web Design -
Typography tokens: I like managing typography using tokens or utilities. Here's an example using Tailwind v4:
@utility typo-header { font-family: var(--font-roboto); font-weight: var(--font-weight-bold); font-size: clamp(2.5rem, 1.556rem + 4.05vw, 3.5rem); line-height: 100%; letter-spacing: 0; }
Hope this helps!
Marked as helpful - @ban-tit
- P@devmd1997What are you most proud of, and what would you do differently next time?
Learning how to properly use the positioning elements really helped out when my components kept resizing.
What challenges did you encounter, and how did you overcome them?The toughest part was trying to get the background overlay to look similar to the design. I tried my best with the background overlay styles but it didn't look exactly like the design.
What specific areas of your project would you like help with?I would like to learn more about how to overlay a color over a background image. It was tough to do with the z indexes.
P@BlonoBuccellatiGreat job! I noticed a couple of things that might be worth considering:
- It looks like the buttons don’t have any hover styles applied.
- Using clamp() for font sizing might help make the typography more fluid and responsive across different screen sizes.
Just sharing some thoughts — hope this helps!
- @yrjebP@BlonoBuccellati
Great job! Sorry, I can't review this without seeing the full code.
- @NameLesSs00What are you most proud of, and what would you do differently next time?
.
What challenges did you encounter, and how did you overcome them?.
What specific areas of your project would you like help with?.
P@BlonoBuccellatiGreat job!
- Using a
<section>tag rather than a generic<div>might improve semantic structure and accessibility.
<section className="md:grid md:grid-cols-3 md:grid-rows-2 md:gap-4 "> {/* Some JSX */} <section className="md:grid md:col-start-2 md:row-start-2">- Also, it would be a good idea to wrap everything with a
<main>tag.
export default function Home() { return ( <main className="flex flex-col p-4 ">Just sharing some thoughts — hope this helps!
- Using a
- P@lordeverP@BlonoBuccellati
Great work!
Regarding the markup for the price:
- Using a
<span>instead of an<h2>might be more appropriate here, since the price is not a heading semantically. Headings like<h2>are typically used to structure content hierarchically, and a price is more of a data point than a section title.
<h2 class="text-preset-1 font-fraunces font-bold text-green-500"> $149.99 </h2>- For the previous price, using
<del>instead of a plain<span>might be a better semantic choice.
The<del>element indicates content that has been removed or is no longer accurate,
which fits well when displaying an outdated or discounted price.
<span class="text-preset-5 text-gray line-through font-medium"> $169.99 </span>Just sharing some thoughts—hope this helps!
- Using a
- P@NunoJDMachadoWhat are you most proud of, and what would you do differently next time?
I'm proud of learning about tailwind custom color classes like text-[<hexcode>]. But maybe next time I'll create css classes so the code is a bit cleaner.
What challenges did you encounter, and how did you overcome them?1 - Struggled with managing the font colour and weight differences in the "Preperation time" card and the "Instructions" section, but after some experimentation it turned out alright.
2 - Also struggled with the table alignment to get it looking as close to the design as possible. A mixture of width and padding classes on the <td> elements seems to have done the trick.
What specific areas of your project would you like help with?I would like some feedback on points 1 and 2 above. Although I managed something I'm not sure if there is a better approach.
Also some feedback about the semantic tags would be helpful. I seem to be abusing <section> a lot. I thought about <article>, but these aren't really articles? dunno
P@BlonoBuccellatiGreat work! As for point 2, Sorry, I'm not sure what the best practice is either.But I had a couple of thoughts:
-
font-[700]corresponds tofont-bold, andfont-[600]tofont-semibold. Both are utility classes provided by default in Tailwind CSS.
So for better readability and maintainability, usingfont-boldorfont-semiboldwhere possible might be preferable. -
The
<hr>element is typically used to indicate a thematic break in content. However, if you're already using<section>elements to define content structure,<hr>might be unnecessary.
In such cases, it may be more appropriate to handle visual separation with CSS instead.
just hoping it might be helpful!
Marked as helpful -
- @codi-AndreP@BlonoBuccellati
Great work! I didn't know about
font-display: swap, so this was really informative.Thank you! I was wondering if it might be better to loop through the links used in the<li>tags using JavaScript's map method or something similar. What do you think?<li> <a className={styles.link} href="http://" target="_blank" rel="noopener noreferrer" > GitHub </a> </li> - @MichelMUNEZEROP@BlonoBuccellati
Sorry. I could not write feedback because I could not find the code.
- P@ecarlsteWhat are you most proud of, and what would you do differently next time?
This is my first challenge on this website so I would have to say the thing I'm most proud of is just getting started. I'm primarily a backend and DevOps/Platform engineer although I've always been drawn to UI work. So I'm just excited to be working on what I've always wished I worked on more.
I typically work in type safe languages and I build most of my hobby backends as well as frontends in TypeScript. I wish I would have converted this repo to TypeScript ahead of time. I also would like to get faster at testing UI as I'm less familiar with frontend testing than I am with backend.
I would also extract the QR code component out into an actual React component that I would pass potentially pass the text and QR code to as props if I were building this to reusable, so I'll be refactoring it.
What challenges did you encounter, and how did you overcome them?The only bit that felt challenging to me about this particular component was how to get the white containing div to size how I wanted it to around the image and get the text to look right. I ended up using the tailwind directive
max-w-xsto get the white container to shrink down and I'm thinking there is likely a better way to handle it than what I've been able to get working.I primarily just spent a little bit of time looking through some tailwind docs to see what would might be able to force the dive to shrink down as tightly as it could around the QR code image.
What specific areas of your project would you like help with?I'm clearly no expert in React, NextJS, or tailwind. I know just enough to get things done, albeit likely not the best way. I'd love to get some feedback around how I organized the usage of CSS mostly, although I'd love to see how someone with more experience with tailwind would have organized the className attributes to achieve this.
I was also a bit unsure about the use of the Google font and how I imported it using
next/font/googleand how I combined it with inlineclassNameuse. I'd love to see how a someone with extensive tailwind experience would do this.P@BlonoBuccellatiGreat work! I'll use this as a reference. Thank you! I noticed that outfit.className is applied twice in your code. If you apply it to the parent <div>, it will automatically be inherited by both child elements, making the code cleaner. Just a small suggestion—what do you think?
// Before <h1 className={`text-slate-900 text-[22px]/7 ${outfit.className} font-bold`}> {/* some JSX */} <p className={`text-slate-500 text-[15px]/5 ${outfit.className}`}> // After <div className={`px-3 pt-1.5 space-y-4 ${outfit.className}`}>Marked as helpful
