My first React and Tailwind challenge

Community feedback

• 

  T

  Alex•2,010

  @AlexKMarshallPosted over 1 year ago

  This is great. And for a first project with React, it all looks idiomatic to me. I certainly agree that form validation can be a bit of a rabbit hole. Using React Hook Form here was the sensible option.

  For matching off against individual parts of the password validation, I think the way to do it is to break up the regex. Have one regex for each criterion, and use that inside a custom validator function. This is with the caveat that I have yet to try it myself.

  Another alternative would be to use a schema library like Zod. That way you can define all your rules, and the failure messages associated with each, in the schema. Then pass the schema to RHF in one place rather than wiring up all the field validators separately. If you were building an app with a lot of forms, I would definitely recommend this. Though for a single form the extra bundle-size might not be worth it.

  A couple of other things I noticed while digging through the code:

  Composition

  In general all the components are nicely composable - accepting children. In the page intro component though there's a mix of a heading text prop, plus children. My personal preference is to avoid those kinds of text props in React. Mainly because it becomes more difficult to pass markup into them. If, for instance, you wanted to wrap a specific word in the heading in a span, you now have to pass in a fragment which feels awkward. I would probably export a PageIntroHeading from the same file and use that to wrap up the h1 styles and let the consumer pass it in

  <PageIntro>
    <PageIntroHeading>Learn to code...</PageIntroHeading>
    <p>See how other dev...</p>
  </PageIntro>
  

  Tailwind class strings

  In the form component, you've extracted the input styling into its own string to be reused in the various fields. I would try to avoid that. Unless you explicitly add config to target named variables, I believe the tailwind compiler will struggle to properly purge unused class names, as it will just look through the jsx for the className prop. That said, it looks ok in the deployed site, so Tailwind may have made the compiler more intelligent at spotting classes wherever they are used.

  I personally would prefer to extract a styled <Input/> component that packages up the class names instead. Or you could use @apply and create a component class for it. At work we tend to do the former so there is only ever one way that a component can be styled, but both are valid options.

  Visually hidden

  One more small thing. You might want to accept an additional as prop in the VisuallyHidden component. That way you can pass in either a div or a span, which would mean this can be used in places where one or the other of those would be invalid HTML. E.g. if you needed to wrap a <p>

  function VisuallyHidden({as = 'span', children}) {
    const Element = as
    return <Element className="sr-only">{children}</Element>
  }
  
  
  

  In general, I try to avoid as props. They can make the components much more complicated. Especially if you try and do tricks like make your buttons work as links etc. But I think here is a good use case for them.

  Marked as helpful

  3

  ---

  

  T

  Grace•31,250

  @grace-snowPosted over 1 year ago

  Thanks @AlexKMarshall ! That’s all really helpful. You’ve picked up on the exact things I had niggles over.

  Think I’ll give this one a break for now but come back and refactor a bit later. I might do a few more simpler (well, non form!) challenges first

  0
• 

  P

  visualdennis•8,375

  @visualdennissPosted over 1 year ago

  Hey Grace,

  its great to see a new submission from you, especially timely for me i'm currently working on this one too using react-hook-form and yup, so it is great for me to have an example solution to look up, compare and explore.

  Very minor thing but it looks like the title is a little bit off, not aligned with the start of paragraph below either, i think margin-left: auto is causing that little push to the side.

  As for the design, i'd like to share my subjective view on it: currently it looks like a bit too busy to my eyes, with around 4-5 different colors/tones used in total on a single page. I assume you gave a background color to left column to increase contrast for readability, but on the other hand right column is using solid color backgrounds while left is semi-transparent, so it seemed a little bit inconsistent to me. So i was thinking perhaps using a solid color, maybe same color as "Try free" box could be an idea. But since the purple tone is too saturated similar to body bg, a more neutral + darker tone could be chosen in that case. Or maybe making "Try free" box in the same style and tone as left column and semi-transparent. I've experimented a bit with colors and styles on dev tools, but couldn't come up with something satisfactory as well, but nevertheless decreasing the number of different tones/colors could be good to make it visually a bit more balanced. Just a little food for thought.

  1

  ---

  

  T

  Grace•31,250

  @grace-snowPosted over 1 year ago

  Thanks @visualdenniss, all really good points!

  I've just pushed an update,

  • using the existing neutral color from the styleguide instead of the opaque varsion
  • adding shadow to match the other boxes
  • Improving the 'thank you' styling

  And good spot on the misalignment! There was a rogue margin auto in there that should have been for mobile only.

  0