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

Remove jquery #777

Open
angrave opened this issue Jan 3, 2024 · 1 comment
Open

Remove jquery #777

angrave opened this issue Jan 3, 2024 · 1 comment
Assignees

Comments

@angrave
Copy link
Collaborator

angrave commented Jan 3, 2024

A search suggests it appears jquery is only used in

./src/screens/Admin/Instructors/InstructorList.jsx
./src/screens/Watch/Utils/keydown.control.js

as a wrapper for getElementById. Once it is no longer needed it remove it from package.json and yarn.lock.

A simple fix would be to replace jquery with native functions. However working directly with html DOM is not the react way of doing things. Is there a better way?

When fixing this, it would be important to fully-understand (and manually check that the arrow key behavior is unchanged) the focus / UI behavior in keydown.control.js i.e.,

Does the implemented behavior work well for users who use a screen reader and keyboard control?

@harsh183 harsh183 self-assigned this Jun 1, 2024
@harsh183
Copy link
Member

harsh183 commented Jun 2, 2024

#806 - first we fix instructor view with a react state

Part 2 with keydown.control.js is a lot more complicated with 58 instances of $. Most of them are finding the element, using length, or focusing, but some have more complicated selectors. It's definitely far from the react way, so I'd recommend

  • writing a fairly extensive set of tests for the Watch screen
  • adding the right state+call backs for various keyboard actions
  • more tests to make sure each action is good

Having jQuery (or even native findElementById) is a code smell, but it's a fairly sizable refactor to fix.

@angrave angrave closed this as completed in ec8c00a Jun 2, 2024
@harsh183 harsh183 reopened this Jun 2, 2024
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

No branches or pull requests

2 participants