React-TypeScript clock app that uses Sass and simple animations

Community feedback

• 

  Steven Toben•750

  @steventobenPosted almost 4 years ago

  Your solution looks really nice imo, but some things in your code definitely confuse me a bit. The composition of your React components definitely throws me off at some places, like the div that has a class name of header and is supposed to take up the entire screen (btw there's a bit of overflow-y on that element. I'd set the height=calc(100vh - 2rem), padding=1rem, and overflow: hidden; if I were you). Also you have a lot of "magic" pixel values throughout your stylesheets which are confusing to understand and will be difficult to maintain. Imo you should set your font-size=16px; in your html selector, and then start using rem units for measurements. Also one more odd thing I found is that you're using sass and even using sass partial stylesheets!! but you're still using normal CSS variables instead of SASS $variables!! Just one last thing to knitpick, personally I would make the quote at the top just a tiny bit darker. Maybe like #d0d0d0 or a slight off white color to make the name really pop. Also it would be cool to see if you could get a keyline set up to align all of your text at around the position it's at now, but with all of the text flush up against a keyline. These are all just little knitpicky things, truthfully this looks great, I just think it could be even better with more transparent component composition and using relative units over absolute lengths. Great job!!

  2

  ---

  

  Vinci Taylaran•185

  @vincitaylaranPosted almost 4 years ago

  @steventoben Thanks!! Going forward with other challenges, I’ll definitely take your advice on not using magic pixel values and using rem units. I just get tired with sticking to one challenge that I start to get lazy and begin to rush to finish the challenge.

  0