Cats-n-coffee

Latest comments

• 

  Mark•210

  @MarksKolbanevsSubmitted over 1 year ago

  

  Rock, paper, scissors game on React.tsx with scss

  #react#sass/scss

  1

  70
  

  Cats-n-coffee•670

  @Cats-n-coffeePosted over 1 year ago

  Hi Mark! Great job on this game challenge! I'll give as much feedback as I can (I'm on FF, 13 inch laptop).

  • the bottom choice button (red choice, fist in normal mode) is hidden by the Advanced and Rules buttons. It could be a layout issue? This results in the red choice being only clickable on its top half, unlike the others which can be clicked entirely.
  • between 700px and 870px the Advanced button overlaps the red choice.
  • when switching to Advanced the bottom 2 options are cut off from the screen, maybe you're missing a vertical scroll?
  • Normal mode in mobile size looks very neat!
  • line 38 of GameChip.tsx could be on multiple lines to make it easier to read.
  • in RoundStarter.tsx, I wonder if you'd be able to make the else if on line 25 easier to read by storing values in a object like this:

  const matches = {
  rock: ['scissors', 'lizard'],
  paper: ['rock', 'spock'],
  // all the rest
  }
  // then in "else if" you can check the key and values
  matches[pickedChip].includes(pickedComputerChip) // this probably doesn't work, but you get the point
  

  • Your overall project structure is good, and easy to follow!

  Let me know if you have questions!

  Marked as helpful

  1
• 

  Nathan Turkson•120

  @nathan-codesSubmitted about 2 years ago

  

  Using the JS DOM to solve the Interactive Rating Component Challenge

  1

  10
  

  Cats-n-coffee•670

  @Cats-n-coffeePosted about 2 years ago

  Hi Nathan!

  Great job for a first DOM manipulation project! I think you could solve your clicked class issue by using <input type="radio" > instead of 5 span elements with class switching. The radio input would give you everything you need for this project with easy way of retrieving the selected value, and easy way to style with CSS (I'm referring to this element, not sure if you're familiar with it, there is also a styling example). I think it would also be better semantics and accessible, since this input is to make the user choose one option only. You can find ways to style these (custom) by googling it, there are a lot of posts about this. I'll give some Js feedback since you said it's your first DOM project:

  • your 4 variables at the top can be const instead of let since you are not re-assigning the variables.
  • to help with your logic as you have it now, you could keep track of the clicked rating in a global variable in your script, and use that number in your submit event handler to update the rating number, that way you don't need to update the DOM before the rating was submitted. Once submitted you can update the rating number element and display the feedback state (so this would be in your submit handler, not in the click handler). The click event would update the rating number (in a variable) and toggle classes. Actually you might want to remove the clicked class on all of them, and add the one class to the selected rating after (remove all, add one). You would not be using toggle in this case, but add and remove class.
  • you can clean your code once your project is finished, remove comments, remove blank lines, ... (only blank lines in your case), it's a good habit to get into if you want to apply for jobs later!

  Great job! Hope this helps, let me know if you have questions!

  0
• 

  Edüardo•370

  @MendesEduardoSubmitted about 2 years ago

  

  Social Media Dashboard in React.js

  #react

  1

  20
  

  Cats-n-coffee•670

  @Cats-n-coffeePosted about 2 years ago

  Hi Eduardo!

  Nice job! To answer your question about code size/readability, I think what you have is good! I would suggest moving the "data" from App.js to its own file which you can import to App.js to use (talking about const dadosCardTop and bottom). If the images cause you issues, you have a few ways to get around the problem. You can pass the name (facebook, instagram, ...) to a reconstructed path which you can reconstruct in the src attr (if you use img), you could try passing the full path to image as an object property, or you could make all the icons in a separate component and pass a prop (I had done this here over a year ago).

  Nice job breaking up the components code with Js and Css on their own, this makes things clean and reusable.

  I believe your theme switcher can be simpler. There are different ways to deal with these, my go-to choice is to get CSS variables on the body to match a data attribute. That way you can set all the variables in CSS, you'll reuse the same variable name for both themes but change the values. In your code you can use the CSS variables and once the data attribute changes (using Js) the colors toggle all at once. (example in the same project)

  Great job! Hope this helps!

  Marked as helpful

  0
• 

  kamil kawa•540

  @GlaDdosSubmitted over 2 years ago

  

  Expenses chart component

  #sass/scss

  1

  31
  

  Cats-n-coffee•670

  @Cats-n-coffeePosted over 2 years ago

  Hi Kamil!

  Your solution looks good but I can't see the chart (using FF)! There is an error in the console this.data is undefined which shows the stack trace from generateBars. I'm not sure if it's because this.data is inside an async function and the result needs to be awaited when using it (or run the next step inside the .then()). I wonder if the load event is finished before you get the result of the loadData function since they seem to run in "parallel"? Your code looks clean, would love to see this canvas work!

  1
• 

  Brodie•110

  @NeoAi12Submitted over 2 years ago

  

  Single Price Card

  1

  30
  

  Cats-n-coffee•670

  @Cats-n-coffeePosted over 2 years ago

  Hi Brodie!

  Nice work! You solution isn't responsive for smaller devices because you hardcoded the width. An easy way to make a card component responsive would be to give it a max-width: 700px (or whatever size/unit you want) and width: 100%. This will help keep your component below a max size while using 100% of the width. For this specific project, you would also need to adjust the layout since the mobile version is aligned as a column with one section below the other. Flexbox might be easier for the purpose of this project, but you can achieve this with Grid as well! Other feedback:

  • You seem to have forgotten the font, I'm not seeing it in the head tag or in your CSS.
  • You can add a hover/focus state to your "Sign up" button and a cursor: pointer, this helps the user seeing where the actions are on the page.
  • Good job picking appropriate tags overall on your HTML!
  • Try to indent your code (HTML in this case), it makes it easier to read and remove unused code such as the main selector in your CSS (it's empty).

  Good job! Hope this helps!

  0
• 

  Lucas Bailo•100

  @lucasbailoSubmitted over 2 years ago

  

  Testing my HTML, CSS anb JavaScript skills!

  1

  20
  

  Cats-n-coffee•670

  @Cats-n-coffeePosted over 2 years ago

  Hi Lucas!

  Very nice work! I'll try to answer as many of your questions as I can:

  • 1 I think radio inputs are appropriate for this problem, good choice! One thought: was the div inside the label really necessary? could you add the container__button class to the label or span instead?
  • 2 Have you tried this?

  .redo__button-container:hover .redo__button-text,
  .redo__button-container:hover .redo__button {
      color: var(--Orange);
  }
  

  • 3 I'm not sure I understand this one. Might be a few things to unpack here. If you're trying to toggle classes to change from review to thank you page, using display: none is probably going to block you. Maybe this can help? https://stackoverflow.com/questions/3331353/transitions-on-the-css-display-property You can also use @keyframes for more fancy animations/transitions. Those are all CSS transitions.
  • 4 In my opinion, you have too many wrapping divs. Elements can be on their own and have wrappers for layout purposes/design requirements (images, bg, ...). You should be able to have your section and img, h2, p and button directly underneath. Only the radio inputs can have a wrapper to make layout easier.
  • 5 Use let or const but do not use var. Since the arrival of let and const there is no need to use var anymore, it causes headaches and makes your code leaky. You can place all your elements selectors at the top (including checkbox). Try to avoid unnecessary blank lines, and clean up your console logs when you're finished with your project (and commented code, but you don't have any). Great job separating functionality with getRating and changePage.
  • 6 Around 760px wide the component, looks really stretched out, maybe add a max-width earlier?

  Nice work! Hope this helps!

  Marked as helpful

  1