Skip to content

feat: added disconnect functionality for connected platforms #84

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

chibuike-19
Copy link
Collaborator

This PR adds the functionality to be able to remove connected platforms from users account

tobySolutions
tobySolutions previously approved these changes Dec 5, 2024
@tobySolutions
Copy link
Owner

Style

  • The code is generally well-formatted, but there are some inconsistencies in indentation and spacing.
  • Some lines of code exceed the recommended 80-100 character limit, making them harder to read.
  • There are no clear coding standards for naming conventions (e.g., handleDisconnect vs OutsideClickListener).
  • Some variable names could be more descriptive (e.g., indx instead of index).

Security

  • The code does not appear to have any major security vulnerabilities, but it is always a good idea to validate user input and ensure proper error handling.
  • The disconnectPlatform function seems to handle errors properly by catching and logging exceptions, but the handleDisconnect function could be improved by adding more specific error messages.
  • It's unclear if the validateTwitch and validateYouTube functions are properly validating user input.

Performance

  • The code uses React Hooks (e.g., useState, useEffect) efficiently, which is good for performance.
  • However, some of the components (e.g., SocialMediaCards) seem to be re-rendered unnecessarily due to state changes in parent components. Using memoization or other optimization techniques could help improve performance.
  • The code fetches data from APIs on component mounts and reloads pages after disconnections, which might lead to unnecessary network requests.

Design

  • The design of the components seems to follow a consistent pattern, with clear separation of concerns between different parts of the application (e.g., layout, routes, API calls).
  • However, some parts of the code could be refactored for better modularity and reusability. For example, the OutsideClickListener function is only used once; it might be worth considering making it a separate module or utility function.
  • There are some minor issues with accessibility (e.g., missing alt attributes on icons), which should be addressed to ensure the application is usable by all users.

Also there's an error in this line:

<button ref={dropdownRef} onClick={() => handleDisconnect("Twitch")}>

In the YouTube disconnect button it says "Twitch" instead of "Youtube".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants