Jonathan
@jchu51All comments
- @youssefsouaSubmitted about 1 year ago@jchu51Posted about 1 year ago
Hi @youssefsoua
Great job on completing this project.
- form Tag: Please use it for the rating; it helps group related inputs.
<form onSubmit={...}> <input type="radio"/> <button type="submit" /> </form>
- Radio Type for Rating : Radio buttons are best for ratings since users can select only one option.
<input type="radio" name="rating" value="1">
- button vs. a Tag : Use button for actions like submitting and a for redirects.
<button type="submit">Submit</button>
0 - @professorkabirSubmitted over 1 year ago@jchu51Posted over 1 year ago
Hi @professorkabir,
Great work!
A few minor suggestions:
- Address the code formatting,
prettier
. - Using media queries for mobile devices; currently, the QR code image appears too large on mobile screens.
@media only screen and (max-width: 600px) { .className { width: ... } }
- It might be beneficial to use 'rem' or 'em' units for font sizes, padding, and margins.
- Instead of relying on element selectors, consider using the class attribute more frequently for CSS styling.
0 - Address the code formatting,
- @CodePawsDevSubmitted over 1 year ago
Actually, I believe it doesn't necessarily require the use of React and Tailwind. I'm practicing with these tools, so I opted to use them. This can be completed easily using HTML and CSS.
@jchu51Posted over 1 year agoHi @CodePawsDev,
Congratulations on completing the challenge! 🎉
Your code is well-organized and neat.
One observation to note: In mobile landscape mode, the blue background doesn't fully cover the height.
0 - @suhjiwon95Submitted over 1 year ago
I'm open to all suggestions! :D
@jchu51Posted over 1 year agoHi @suhjiwon95,
congrats on completing the challenge. 🎉
few minor suggestions It's a best practice to call API endpoints using the async/await pattern with promises. To handle potential failures, always include try-catch blocks. For a better user experience, consider adding a loading status or a failure message. While data is being fetched, display a loading indicator or text in the box. If the fetch fails, a message such as 'Failed to generate, please retry' would be beneficial."
It's advantageous to explore various frontend frameworks like React, Vue, and Angular. Additionally, if you're utilizing VSCode, consider installing 'prettier' for enhanced formatting.
1 - @timilehin223Submitted over 1 year ago
Anything I could have done better?
@jchu51Posted over 1 year agoLGTM.
-
It's great to see you're using
picture
tag, to handle image -
Try to avoid using IDs / elements selector for CSS styling. Class selectors are generally preferable as they're reusable and don't have the high specificity that IDs have, reducing the risk of style conflicts.
-
Use rem for consistent sizing across your website. The sizes are based on the base font-size, and they don't change with the parent element's font-size.
2 -
- @suhjiwon95Submitted over 1 year ago
Do you have any tips for getting the margin right?
@jchu51Posted over 1 year agoIt's great to see you're using
rem
units for padding. To enhance your CSS abilities further, consider creating files likevariable.css
andmedia.css
. These could be useful across many projects. Additionally, familiarizing yourself with Sass (Syntactically Awesome Style Sheets) could be very beneficial.I'd also recommend getting to grips with the BEM (Block, Element, Modifier) methodology. It's a popular naming convention for classes in HTML and CSS.
A few minor suggestions for improvement include:
- Working on your code formatting to improve readability and maintainability.
- Limiting the use of element selectors. This could prevent potential CSS conflicts in larger projects.
- From an accessibility perspective, include alt text in your img tags. Not only does this improve accessibility for those using screen readers, but it also provides a text descriptor when images can't be loaded.
- Try to steer clear of negative values in your CSS positioning. This can sometimes cause unexpected issues.
- For responsive design and performance, consider using the picture tag for providing different images for different device sizes. Alternatively, manage the scaling of images using the width and height attributes within the img tag.
Marked as helpful0