Dine challenge with React

Community feedback

• 

  Steven Toben•750

  @steventoben

  Posted over 4 years ago

  Hey this looks great I'll answer your questions first. For the first questions, yes you can do that. I personally do that for my Typography.jsx components. It basically looks like this:

  const Typography = ({as, children, ...props}) => { return React.createElement(${as}, props, children); }

  So you could apply the same logic to a button. Basically what I'm doing is creating a functional component with the props de-structured, so you'll have access to the 'as' prop, which is just a string of what element you want to create. You also have access to the children and all other props passed. So <Typography as={'h2'} className={"Display4"}> Some text </Typography> would come out rendered to the dom as <h2 class="Display4">Some Text</h2>. So that's an approach I like using, you could just use an if statement and check if the prop exists. I have a React UI component library and my Buttons are extremely customizable and reusable, reusable code and styles is such an important part of front-end development.

  For resetting your form you could create a ref for your form DOM node and then you would be able to access the form on the actual DOM and call methods. So you could do ref.current.reset(); If you're using React state for the values you want to reset, you could simply just set the state to it's initial value, you could keep an initial value in a ref (instance variable kind, not DOM node kind) I couldn't quite tell how you were controlling the form's state.

  It looks like you're attempting to update the value of the image in a function in your functional component. You'll need to put that function inside of a React.useEffect hook, and i'm guessing the dependency array would be window.innerWidth, unless you're rendering from the server. Try putting it in a useEffect hook like I said so basically every time the window width changes, it will call that function and check with the new value. You could also wrap it in a Memoized function since you only want to calculate if the one variable changes, but I'd go for the useEffect approach first.

  Ok sorry I'm not exactly sure what you mean by this one. Not sure how much help I can be with this one but off the top of my head if it was rendering a square but you wanted a curved part then I'd try to set the clip-path property on it's class or the mask-image. Clip-path is really useful and I use it quite a bit, it let's you make shapes out of your box model, you can do circles, ellipses, polygons, and I think arcs. There's a lot you can do with that styling rule so maybe it could work, I'm not too sure sorry tho.

  So yeah now onto just some general stuff, first off don't set font-size to 62.5% in your html tag. I have to tell people this every single project lmao but you should either set font-size to 100% or don't add the rule and let it be inherited from the browser settings (either way it should default to 16px). I know people use it so that 1em=10px but really, 16px is better anyways plus you don't mess with the user's accessibility settings. Also i find it kinda odd that you're using sass, you even made a partial for variables, but you're using standard css variables lol. Also just a quick typography lesson because every single person uses the properties incorrectly; font-size (unless you're initializing root size to 100%) should always set in rem units and idealy you should have a typographic scale set up but that's kinda overkill for these small challenges. Letter-spacing should be set with em units. Line-height should be unitless. It's a multiplier of the font-size, typically paragraphs have a line-height around 1.5, and headings are generally lower at around 1-1.25. Also I would try to avoid setting font-styles to tags like h1 or p, because usually you'll have many different sets of headings or body classes, and you don't want to be that broad with selectors. Also good jobs measuring all of your widths and heights and paddings and stuff in rem units, that's good practice. I don't know where a lot of the sizes are coming from tho, there seems to be a lot of magic numbers in there. This is just a personal design rule but I try to make sure I never see a number in my sass. If it's an important number it will be a variable, if it's not in my set of variables (which are limited and meaningful, which lets my pages have vertical rhythm and a sense of harmony) then there's probably a better way of sizing or positioning something. But that's just me being pretentious because I like UX lol. The last thing is I recommend to try to reduce the amount of nesting your do with sass. Nesting too much can make your compiled css file size huge.

  Oh and one last thing, there definitely are annoying html tags that won't let you restyle them, especially the select and options. That doesn't mean you can't make your own custom dropdown tho. I have like 10 different variants of dropdowns, you just have to make sure to correctly set the attributes saying that it's a select or an option. I would definitely have done that in your case because mapping out all of those options would have been so simple compared to listing them all out. I often make custom textfields too because they always look much better if I have a fake placeholder element and a label element that animate, it increases people's interest quite a but. Sorry I rambled so much lol this was just a pretty great looking project and it uses React which I love lol. Great job tho I think overall this looks great. Good job!! Let me know if you have any questions!

  320
• 

  

  Matt Studdert•13,611

  @mattstuddert

  Posted over 4 years ago

  Hey Em! You've already got a bunch of detailed and useful feedback, so I'll just say brilliant work! 💯

  As you work with React more, you'll probably move towards making your SCSS files map 1:1 with your components (plus a bunch of global styles). This is a great architecture when working with component-based libraries/frameworks and is essentially why solutions like Styled Components work so well with React.

  Keep up the great work! 🙌

  22
• 

  

  Grace•32,130

  @grace-snow

  Posted over 4 years ago

  @emestabillo I think this solution looks fab and you've done a great job here!!!

  A few tiny recommendations still for best practice in accessibility (apart from the old font size discussions above!) is as follows:

  • focus highlight is really subtle. I get why and this is a classy looking site. So maybe consider having a play with the :focus-visible pseudo selector in css. That could allow you to give a much more obvious focus state for keyboard users, while keeping a subtle and stylish focus style for anyone else.
  • In your form I recommend making those date grid divs into a fieldset, letting the field info be the legend (no paragraph tag required) and using aria-label attributes on each date and time field instead of the separate label. I tried it with Voiceover and it just reads a lot nicer and makes it really clear that the 'pick a date' has 3 related inputs.

  I've often used sr-only labels instead of aria-label and they work fine in most circumstances. But they are read out separately, so in groups of fields like this it really slows down the reading.

  Minor things really, and by no means essential, but I thought you might enjoy the extra suggestions

  22
• 

  

  Emmilie Estabillo•5,580

  @emestabillo

  Posted over 4 years ago

  @steventoben You seem pretty opinionated in your ways 🙂. Show and tell works great for me. It would be awesome to wrap all your comments and skills into a project from FEM so we can have a look and study the structure you seem so passionate about.

  I understand every team is different and have their own style guides. I have no problems conforming to their set of style rules when I join one. Not sure I’m FAANG material just yet, I’m about 20yrs experience short lol. So I might still use the percentage route with base font. For now, and for portfolio purposes.

  110