Interactive Comment Section using Next and TypeScript

Community feedback

• 

  StarWolf•260

  @BParadowskiPosted about 1 year ago

  Okay, after taking a look at your code I have some suggestions.

  • Naming: your names are not descriptive. To be frank I have no idea what most of your code does. Component name like "ActionButton.tsx" doesn't say anything about its purpose (every button has some action/function after all). The same goes for props the like of "isReplyReply" or "onIsReplyingChange".

  • Continuing in the spirit of code readability: destructuring assignment. It allows us to do stuff like changing this:

  const onCommentVoteChange = props.onCommentVoteChange;
  const commentIdToChangeVote = props.commentIdToChangeVote;
  const onReplyVoteChange = props.onReplyVoteChange;
  const replyIdToChangeVote = props.replyIdToChangeVote;
  

  into this:

  const {
    score,
    onCommentVoteChange,
    commentIdToChangeVote,
    onReplyVoteChange,
    replyIdToChangeVote,
  } = props;
  

  • Components: you have 4 different button components which fundamentally are only visually different. A more sensible approach would be to create one component with variants.

  • Lastly, prop drilling and logic placement. There is no reason to define comment adding logic on the button! There is also no reason to make delete modal a child of the delete button... Aim to place the logic where it makes sense.

  If I were you I would focus mostly on code-readability and fighting your urge to prop-drill. I must confess that I still have no idea what is going on in most of your code.

  Marked as helpful

  1

  ---

  

  Taufiq•150

  @taufiqmahdiPosted about 1 year ago

  @BParadowski

  First, I want to say thank you so much for giving me the feedback on this challenge.

  • Naming: Thank you for the feedback, I thought myself that that has been the best way to describe what something does, turns out is not, that's what experience teaches you I think. I will work on it in the future.
  • Continuing in the spirit of code readability: I know! I was thinking there must be an easier way to do this, turns out there is. I will apply this in my future projects!
  • Components: Thats a great idea! But i wasnt sure how to implement it. Also, they had different functionality so thats why i was thinking to differentiate it. But maybe, give it a props that say what is the variant and invoke the function based on the variant, right?
  • Prop drilling and logic placement: Yeah... But i dont know how to place the function that is when the button is clicked, the function in two upper component invoked... Thats why I do that.. Yeah thats correct too... Maybe the solution is to place the correct calling function in the correct component, right?

  Overall I see your point, thank you so much for the feedback, im glad you spend your time giving me a feedback :D

  1

  

  StarWolf•260

  @BParadowskiPosted about 1 year ago

  @taufiqmahdi

  With regards to components: Another thing worth mentioning is that you do not need to make every a component (nor should you). When thinking about dividing your app into components ask yourself questions like:

  • Does this piece of code encapsulate some logic/function?
  • Am I going to reuse it in different places?
  • Does moving this piece of code into another file greatly improve code-readability?

  or, once you understand RSC and Next 13 app directory:

  • Is this an island of client-side code on otherwise server-side generated html?

  If the answer to one of these is yes, then consider making it a component. For example, when looking at this project I would think mainly about 2 components:

  • <Comment> component which optionally recursively renders replies to it and a new comment form.
  • <NewCommentForm> self explanatory

  This would also answer you questions about logic placement. Where should new comment placing logic go? into the form. etc.

  I also think that many problems in this challenge stem from you trying to simulate the backed. In reality this thing would be a bit complicated (involving authentication and database syncing with something like ReactQuery). I suggest you take a step back, understand design patterns of React and UI first and only then move to learning those, for real this time.

  Happy coding!

  Marked as helpful

  1

  

  Taufiq•150

  @taufiqmahdiPosted about 1 year ago

  @BParadowski

  Thank you again for the feedback.

  True that, I find myself stuck that I think this component is used few times but in different depth of components, that is where I am confused.

  I see, maybe I was making everything a component as small and as detailed as possible, which should have been not.

  Thank you for the suggestion, where can I learn that? I was thinking at this stage of my understanding in Web Development, the more I code then the more I learn, just a matter of time. Was it wrong or should I work on my basic fundamental first? Thank you

  0