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

Add tests for existing key press dispatches and remove one jQuery call for volume focus (2/x) #839

Open
wants to merge 19 commits into
base: staging
Choose a base branch
from

Conversation

harsh183
Copy link
Member

@harsh183 harsh183 commented Feb 17, 2025

#777

  • Adding tests for the existing dispatch based key interactions
  • Moved down arrow key to focus on volume button as an example of getting rid of jQuery. I experimented with a lot of different ways including effect saga, useElementById,useImperativeHandle, PlayerData global variable but this felt the neatest.

@@ -28,6 +28,17 @@ function VolumeControl({ muted = false, volume = true, dispatch }) {
}
};

const sliderRef = useRef();
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so there isn't a super elegant way to get around findElementById or $ when we want to mess with focus but useRef + useEffect with creating a function available to the entire window is a workaround of sorts.

https://www.reddit.com/r/react/comments/1b5nwoh/focusing_on_elements_programmatically_in_react/ - others seem to also converge around this

@harsh183 harsh183 changed the title Draft: Cleanup keydown control jQuery (2/x) Add tests for existing key press dispatches and remove one jQuery call for volume focus (2/x) Feb 17, 2025
@harsh183 harsh183 mentioned this pull request Feb 17, 2025
@harsh183 harsh183 marked this pull request as ready for review February 17, 2025 07:37
@harsh183 harsh183 requested a review from angrave February 17, 2025 07:37
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.

1 participant