Skip to content
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

Feature flagged mouse movement collection #922

Merged
merged 20 commits into from
Dec 20, 2023
Merged

Feature flagged mouse movement collection #922

merged 20 commits into from
Dec 20, 2023

Conversation

HughParry
Copy link
Contributor

No description provided.

Copy link

Pull Request Review

Hey there! 👋 Here's a summary of the previous results for the pull request review. Let's dive in!

Changes

  1. Added import statement for Collector and StoredEvents in Procaptcha.tsx.
  2. Added sendData state variable in useProcaptcha hook.
  3. Added Collector component in Procaptcha component.

Suggestions

  1. In Procaptcha.tsx, line 29: Import statement for darkTheme and lightTheme is not used.
  2. In Procaptcha.tsx, line 103: Unused variable styleWidth.
  3. In Procaptcha.tsx, line 150: Unused import statement for Modal.
  4. In Procaptcha.tsx, line 271: Unused div element.
  5. In Procaptcha.tsx, line 271: sendData prop is passed to Collector component, but state.showModal should be used instead.
  6. In collector.tsx, line 7: StoredEvents type is not imported.
  7. In collector.tsx, line 41: Unused import statement for MutableRefObject.
  8. In collector.tsx, line 41: Unused import statement for useRef.
  9. In collector.tsx, line 41: Unused import statement for useState.
  10. In collector.tsx, line 41: Unused import statement for startCollector.
  11. In collector.tsx, line 41: Unused import statement for ProsopoKeyboardEvent.
  12. In collector.tsx, line 41: Unused import statement for ProsopoMouseEvent.
  13. In collector.tsx, line 41: Unused import statement for ProsopoTouchEvent.
  14. In collector.tsx, line 41: Unused import statement for HTMLDivElement.
  15. In collector.tsx, line 41: Unused import statement for useEffect.
  16. In collector.tsx, line 41: Unused import statement for useRef.
  17. In collector.tsx, line 41: Unused import statement for useState.
  18. In collector.tsx, line 41: Unused import statement for StoredEvents.
  19. In collector.tsx, line 41: Unused import statement for ref.
  20. In collector.tsx, line 41: Unused import statement for setStoredMouseEvents.
  21. In collector.tsx, line 41: Unused import statement for setStoredTouchEvents.
  22. In collector.tsx, line 41: Unused import statement for setStoredKeyboardEvents.
  23. In collector.tsx, line 41: Unused import statement for ref.
  24. In collector.tsx, line 41: Unused import statement for useEffect.
  25. In collector.tsx, line 41: Unused import statement for onProcessData.
  26. In collector.tsx, line 41: Unused import statement for sendData.
  27. In collector.tsx, line 41: Unused import statement for return.

Bugs

  1. In the logTouchEvent function, the touch variable is accessed without null-checking, which could lead to runtime errors if touch is undefined.
  2. The findContainingForm function assumes that the nearest parent form element will always be found, but it does not handle the case where no form element is found. This could result in unexpected behavior or errors.

Improvements

  • Refactoring suggestion for better readability in collector.ts:
    • In the startCollector function, instead of attaching multiple event listeners individually, consider using a loop to iterate over an array of event types and attach the listeners dynamically. This would reduce code duplication and make it easier to add or remove event types in the future.
    • Refactored code snippet:
      const eventTypes = ['mousemove', 'keydown', 'keyup', 'touchstart', 'touchend', 'touchcancel', 'touchmove'];
      eventTypes.forEach((eventType) => {
        form.addEventListener(eventType, (e) => {
          switch (eventType) {
            case 'mousemove':
              logMouseEvent(e, setStoredMouseEvents);
              break;
            case 'keydown':
            case 'keyup':
              logKeyboardEvent(e, setStoredKeyboardEvents);
              break;
            case 'touchstart':
            case 'touchend':
            case 'touchcancel':
            case 'touchmove':
              logTouchEvent(e, setStoredTouchEvents);
              break;
          }
        });
      });
    • This refactoring improves readability by reducing code duplication and making it easier to understand the logic of attaching event listeners.

Rating

🌟🌟🌟🌟🌟🌟🌟

The code has been rated 7 out of 10 based on criteria such as readability, performance, and security.

That's it for the summary! Let me know if you need any further assistance. Happy coding! 😄

@forgetso forgetso merged commit b57ccd6 into main Dec 20, 2023
6 checks passed
@forgetso forgetso deleted the collector branch December 20, 2023 16:09
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