Jaylen Baxter
@JYLNAll comments
- @hamidriaz1998Submitted 2 months ago@JYLNPosted 6 days ago
This looks great! Here are a few things I noticed that you might improve on:
- In some portions of your code, you have some sizes that are in static pixels rather than a intrinsic size. For example,
max-w-[500px]
on the dashboard container element andtext-[18px]
for the heading of the cards. These static pixel sizes will cause the layout to be inconsistent if a user zooms in on your page or uses a different default font size in their browser. Consider converting these to rems. - In Firefox (at least), the background that is set for the icons is creeping around the corners of the background of portion of the card that has text. You may consider reducing the height of the element that contains the icons to keep this from happening.
- You're selecting the dashboard container element using a class, while the class isn't being used for any styling. I would think that this generally isn't best practice. Consider changing this to an ID for the element instead to use within the query selector of your JavaScript.
- While you have breakpoints for most of your container elements, you don't have any for the text so there is a lot of unnecessary white space due to the fact that your font sizes aren't scaling with the layout. You may consider going back through your code and adjusting font sizes at some of the same breakpoints you have within your code.
- The icon SVGs within the headers of the cards are decorative. You may consider setting the alts of the SVGs to an empty string due to this.
- Within your JavaScript code at line
43
, you can accomplish saving the data from the fetch without creating a separate empty variable. To accomplish this, you can simply returndataJson
in the last .then chain, and then set the variable to the whole fetch operation. Just something that might help consolidate your code a little bit.
Overall, great work!
0 - In some portions of your code, you have some sizes that are in static pixels rather than a intrinsic size. For example,
- @lauraamiaaSubmitted 29 days agoWhat are you most proud of, and what would you do differently next time?
I added some JavaScript validation to the form. In addition to showing the success message, I have also added a loading spinner to simulate processing time. The success message now dynamically displays the actual email address that the user typed into the input field, giving a more personalized confirmation.
The validation is fairly simple at this stage, relying on basic client-side checks using regular expressions to ensure the email format is valid. In the future, I plan to explore more robust validation techniques, such as integrating API calls or database connections to check if the email already exists or to handle server-side form submissions. This would also allow for more advanced error handling and feedback.
What challenges did you encounter, and how did you overcome them?I am still learning Tailwind CSS, and realised if the classes are not compiled in the output, they are not being read correctly. I am working with adding and removing classed using JS for error messaging. So I had to add some in the safelist of the Tailwind config.
@JYLNPosted 8 days agoThis is great! I especially like the loading spinner addition to the project. Here are a view things I noticed that you could potentially improve on:
- The form should not include the
novalidate
attribute for a few reasons. The client side validation improves accessibility for those using screen readers, provides instant feedback to users if values are incorrect or missing if required, and still provides validation for your form if JavaScript is disabled in the browser or fails to load. - You may consider setting the form button's
type
attribute tosubmit
as it will improve accessibility within the form and avoids ambiguity with the form's default behavior. - You may consider setting the focus state of the form button to the same or slightly differing styles as the hover state so as to provide accessibility to keyboard users so it is clear they have navigated to the submit button.
- You may consider setting
min-h
on your main element rather thanh
as when the mobile layout is in landspace, if the vertical space is narrow, the image overflows the top and becomes mostly not visible. - You may consider writing slightly more descriptive and on-topic alts for your images as to improve accessibility with the images. Also, decorative images should have an empty alt. In the case of this project, the check mark within the success modal and within the list of the main text is more than likely decorative and wouldn't take an alt.
- As far as your repo is concerned, I would recommend that you don't ignore the tailwind config file, as if you were to need to clone this repo to another machine, unless you remember your tailwind config, trying to develop more on this project will result in errors. Also, I would recommend that you do ignore
.DS_Store
files as they are non-essential and create unnecessary clutter within your repo.
Overall, great work! You replicated the design very well.
0 - The form should not include the
- @Rahmonbek-0001Submitted about 2 months ago@JYLNPosted 9 days ago
Great start! Here are a few things I noticed that you might improve upon:
- The code in your repo (i.e. your HTML markup and Tailwind utility classes) doesn't match what's being compiled and shown in your preview site. You may consider updating the code in your repo so that you can receive substantial feedback on your practices within your actual source code, rather than what's being compiled for the browser.
- Consider publishing your code for challenges in separate repos rather than a monorepo. A monorepo might get a little hard to handle when it comes to managing revisions.
- When clicking the share button on mobile screens, when you overlay becomes visible, there's a strange layout shift with the card that might not be beneficial to UX. You may consider playing with padding and maybe reorganizing parent elements to get your overlay to line up more precisely.
- Your share icons within the tooltip and mobile overlay should be encompassed by links (even if for the example they don't redirect anywhere) as an img is not inherently an interactive element.
- Your share icons are rendering strangely, as if they are being squished. Instead consider the use of an aspect-ration utility class or allowing the icons to be responsive so that they maintain their aspect ratio.
- You're missing the Manrope font that was specified for the design. Look over the style-guide to ensure you are using the correct resources.
- Your tooltip's arrow and body doesn't appear to be aligning correctly with the share button. You may try playing with the margins you have set on this to get it closer, you might even look into JavaScript libraries that use a floating UI concept that will assist you with this automatically.
These are just a few things that might help make this project stand out more and conform to the design requirements a bit more. Great work overall, however!
Marked as helpful1 - @hikawiSubmitted 3 months agoWhat are you most proud of, and what would you do differently next time?
I'm proud of the entire thing as a whole, this was a step up in difficulty compared to previous challenges for me, and also proud of my inability to double check, see below lol.
What challenges did you encounter, and how did you overcome them?I thought I had to align the two images for hero left and right well to scale up even if it's two separate images, until I realized there's the combined image in the assets folder also. I also had trouble putting the background image in for the footer, until I realized I was misspelling the css property.
What specific areas of your project would you like help with?I saw on a few notes about shorthand properties and the / in css, I dont know what it is, it seems very specific on what it does on specific properties, and MDN docs weren't exactly easy to understand.
@JYLNPosted 11 days agoThis looks great! Good work! Svelte is an awesome framework, I've had a hard time getting into Astro though.
The only thing I can say that may help you in the future, for the buttons in the hero, you can use flex-wrap rather than using flex-col and flex-row with a breakpoint to produce the same result of the column/row layout for the buttons.
Good work!
Marked as helpful0 - @MahdyrllSubmitted 17 days ago@JYLNPosted 13 days ago
This looks great! I noticed some things you might look at improving:
- The breakpoint for the single column grid and 3 column grid is too small. When resizing the browser, content is overflowing the cards that only span 1 column and words are breaking at one line a piece. Also, the quotation SVG on the first card ends up floating over the user information content. It's happening generally around where some tablet portrait sizes exist. You may consider breaking into the single column layout so that your layout is more consistent.
- You have your text sizes set to static pixels, which will cause issues on users that have have their browser settings set to a different font sizes than default. Consider using rems or ems to better your UX.
- On line 28 of your commentComponent, you are using these utility classes:
sm:pl-[10.5rem] sm:pr-[10.5rem]
. Since both values are the same size for left/right padding, you can use this class:sm:px-[10.5rem]
to help clean up the amount of class names you have set within your markup. - Within your
index.css
file, you are setting border-box for box-sizing on all elements. This isn't necessary, as the pre-flight for Tailwind CSS already accomplishes this. You may refresh yourself on the Tailwind docs about it's defaults to help reduce redundant code within your base.
Overall, great work!
Marked as helpful1 - @rafbar2000rrSubmitted 25 days agoWhat are you most proud of, and what would you do differently next time?
I am proud of learning responsive design. I would try to improve my code, making it cleaner.
What challenges did you encounter, and how did you overcome them?It was a little difficult to arrange the cards when the screen size crossed the breakpoint, I overcame by using flexbox resources.
What specific areas of your project would you like help with?I would like to know if there is a better way to code for the cards arrangements when the screen crosses the breakpoint from mobile to desktop. I did it using flexbox but I am not sure if using grid could be easier.
@JYLNPosted 13 days agoOverall, this is a great start! Here are a few things I noticed that you could potentially improve upon:
- You're using static pixels for the majority of your sizes. When a user is zoomed in or has their browser configured with a different font-size, your layout may look strange. You may consider converting your sizes into variable sizes (rems or ems) to allow for more consistent layout in these cases.
- Your icons within the cards are not positioned correctly according to the design. It appears you attempted to position them with the
justify-self-end
utility class, however the parent element for the card is not using thegrid
utility class to allow the utility class to position your icon. You may want to refer to the Tailwind docs and choose your best approach with either grid or flexbox, then update the cards. - You may consider adding padding to your
body
element as when the screen resizes, your content is smashing into the sides of the viewport. - You may consider only setting bottom margin on elements when you're attempting to space out your elements. I usually find it allows me more control over where elements are positioning themselves after the spacing when using this method rather than setting a top and bottom margin on intersecting elements. Likely just a preference though.
- As accessibility is concerned, I've learned it's best practice to make the
alt
attribute on your images as descriptive and specific as possible if the image is not a decorative image (otherwise you'd leave the alt empty). I would say in this case, the icons are decorative, but if you choose to leave thealt
, make sure they are more descriptive as screen readers will utilize your alt tag to describe the image. - I noticed you're linking some other Google fonts that you are not using within the project. You may consider refactoring your links so that just Poppins is being loaded as that's the primary font that's being used. Browsers will use whichever sans-serif font is selected in their settings as fallback rather than what else is linked unless specified.
These are just a few things I noticed and figured I could comment on based on the learning and feedback I've received on previous challenges. Great work!
Marked as helpful0 - @vuminhtruongSubmitted over 1 year ago@JYLNPosted about 2 months ago
This is a good start! However, there are quite a few things you can do to your HTML markup to improve this:
-
The point of this challenge is to practice different layouts on different screen sizes. Your solution simply shrinks and becomes difficult to view on smaller screen sizes. Consider utilizing a custom breakpoint or Tailwind's default breakpoints to help shape a better layout and viewing experience for the user when viewing this from a non-desktop screen.
-
Consider using semantic HTML tags to assist with accessibility and document outlining. For example, this card could be in an
article
element. -
Your prices are using heading elements, but there's no other context for them to be headings. Consider using a
p
element or aspan
. -
Consider giving your overall container a min-height rather than a static height. This will help with responsiveness and will not cause things to be cut off if you have overflow.
-
You have unnecessary classes within your markup. For example, you have
img-container
and 'design-container` as a class, but there's no styles defined in your markup or CSS for these classes. You should define styles under these classes or remove them from your markup. -
You're missing the other font from this design. The style guide has a link to the font, but there are other resources like fontsource that has this font.
-
Your image has the wrong border radius. It's rounding its corners at the top of the element and does not fit the design
There are more things I could point out, but these are just a few things you can do to improve on your solution. Good start again!
0 -
- @GabrielJesusSSubmitted 4 months ago@JYLNPosted about 2 months ago
This looks great!
The only suggestion I would have is potentially remove the div container surrounding your sections below the preparation time, and instead add individual margins within the section elements themselves.
Otherwise, this looks great, good work!
0 - @codebyfauzanSubmitted about 2 months agoWhat are you most proud of, and what would you do differently next time?
I can code faster for this challenge than previous challenge
What challenges did you encounter, and how did you overcome them?didn't meet any of them
What specific areas of your project would you like help with?I wanted to know the best practice of project structure tailwind css standalone
@JYLNPosted about 2 months agoGreat work on this! I have just a few suggestions looking over your code:
-
A lot of the default spacing values of Tailwind fits well with this project given the design constraints, so you can get away with using a lot less arbitrary values in your code. For example, you have
text-[1.125rem]
andtext-[0.75rem]
classes for your font size values. This design works well with the default sizes provided with thetext-2xl
andtext-sm
classes, both of which may help with some of the spacing and sizing that's missing between your solution and the design. -
You have one set width for your social links card. You may consider using the Tailwind breakpoints to adjust the width of the card based on different screen sizes
Great work configuring your Tailwind config! With these challenges, I've gotten a lot more practice configuring Tailwind to fit specific needs and do more things that I need it to farther than the default that's provided.
Marked as helpful1 -
- @vanshraheja75Submitted 4 months agoWhat are you most proud of, and what would you do differently next time?
it was an easy one
What challenges did you encounter, and how did you overcome them?no challenges
What specific areas of your project would you like help with?any suggestion acceppted
@JYLNPosted 4 months agoI like your use of
min-height
and flexbox for centering content. It was a trick I learned way too late into my learning, and it's been a life-saver with centering content. I also have used grid with the same min-height technique and it's great!The only suggestions I have is:
- Be careful to constrain your heights on your elements. Your card height is locked in at a px specific height, especially since it's not
min-height
. It's a habit I have as well. Ultimately, I think it's best that the flow of the layout justifies the heights of the overlying elements. - Some left and right padding on the text of the card may help it not seem so cramped within the card
Overall, great job! Keep up the good work
Marked as helpful0 - Be careful to constrain your heights on your elements. Your card height is locked in at a px specific height, especially since it's not
- @MesrouaDjamelSubmitted 4 months ago@JYLNPosted 4 months ago
Great work on your solution! Even though this is a simple project, I think it's great practice for using and understanding Next, so I commend you there.
A few things I noticed while looking through your code that could potentially use some improvement:
- You used explicit arbitrary values for your sizes throughout the project. Ideally, I would recommend that you use relative values (rem/em) so that your sizes scale properly based on different root base sizes. As well, I would recommend that you stick closer to Tailwind's utility classes (at least with this project, they fit rather nicely). I usually only revert to using arbitrary values in Tailwind classes if none of their provided utility classes produces the result I want.
- Your component could use a bit more spacing and variety of font sizes to fit the design slightly better. For example, I think that the root card element could use a bit of extra padding, there could be more gap between the content elements of the card, and I think the
Learning
tag and published date could be a little smaller. The Figma design file provided with this project gives some great insight on what that spacing and sizes could look like.
Overall, keep up the good work!
Marked as helpful0 - @MattPahutaSubmitted 4 months agoWhat are you most proud of, and what would you do differently next time?
A straightforward component that required minimal markup and styles. I've enjoyed working on these smaller projects to focus on fundamentals and be reminded that there's value in keeping solutions simple and easy to understand.
What challenges did you encounter, and how did you overcome them?Even with this simple component I initially added more markup than was necessary. It felt good tearing things down to the studs.
@JYLNPosted 4 months agoI think your solution is great! I completely understand your statement regarding overcomplicating the markup (I do it a lot too), it's fairly easy to do but good on you for reviewing and refactoring upon that.
I also like the simplicity of how your CSS file is laid out with the variables, reset, and base styles. I especially enjoy the comments of your variables containing the sizes of your intrinsic units in pixels. It shows you paid close attention to detail regarding the design spec.
Great work!
Marked as helpful0