final #1979
final #1979
Conversation
There was a problem hiding this comment.
Your implementation is almost there! 🎉 However, there's a critical issue with the event listener in App.tsx that needs to be addressed. The removeEventListener method is not removing the listener correctly because a new function reference is being used. To fix this, define the event handler as a class method or property to ensure the same reference is used. Once this is resolved, your code will be in great shape! Keep up the good work! 😊
✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨
🔄 Fix the issues, commit the changes, and then re-request my review using the Re-request review button in the Reviewers tab.
src/App.tsx
Outdated
| componentWillUnmount() { | ||
| document.removeEventListener('keyup', (event: KeyboardEvent) => { | ||
| this.setState({ pressedKey: event.key }); | ||
| }); |
There was a problem hiding this comment.
The removeEventListener method is not removing the event listener correctly. You are passing a new function to removeEventListener, which does not match the original function added in componentDidMount. To fix this, you should define the event handler as a class method or a property and use the same reference in both addEventListener and removeEventListener.
There was a problem hiding this comment.
Great job on your project! 🎉 While your code is mostly in good shape, there are a few critical issues that need attention:
-
Event Listener Issue: In
App.tsx, theremoveEventListenermethod is not functioning correctly because a new function reference is being used. To resolve this, define the event handler as a class method or property to ensure the same reference is used for both adding and removing the event listener. -
Naming Convention: The method
stateSetdoes not follow the naming convention specified in the checklist. Method names should start with a verb for clarity and consistency. -
Readability: The ternary operator in the
rendermethod should be formatted according to readability guidelines, with each operand on a separate line.
Despite these issues, your project meets all other requirements and constraints. The decision to approve is made to help you move forward with your learning journey. Please take the time to review the comments and improve your code. Addressing these critical issues will enhance the functionality and maintainability of your code. Keep up the great work and continue learning from these experiences! 🚀
✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨
| document.removeEventListener('keyup', (event: KeyboardEvent) => { | ||
| this.stateSet(event); | ||
| }); |
There was a problem hiding this comment.
The removeEventListener method is not removing the listener correctly because a new function reference is being used. You should define the event handler as a class method or property to ensure the same reference is used in both addEventListener and removeEventListener. This will fix the issue with the event listener not being removed properly.
No description provided.