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

fixes #33 - set a term(s) status via the keyboard #52

Merged
merged 1 commit into from
Dec 11, 2023
Merged

fixes #33 - set a term(s) status via the keyboard #52

merged 1 commit into from
Dec 11, 2023

Conversation

robby1066
Copy link
Contributor

This PR makes the following changes to /static/js/lute.js:

Adds a new function called increment_status_for_marked_elements

This function collects selected term elements, groups them by their data_status_class level, then submits them in batches to be updated with update_status_for_elements.

This function accepts a single argument shiftBy, an integer indicating the amount and direction to increment the status class as defined in the constant validStatuses.

Abstracts the code in update_status_for_marked_elements into a new generic function update_status_for_elements

Separating the selection of elements from the code that submits them to the server allows other functions to update statuses for different use cases.

update_status_for_elements takes two arguments

  • new_status: the status that will be set on the elements
  • elements: a nodeList, array, or jQuery selection of elements to update

Passes a new argument to update_selected_statuses

update_selected_statuses was doing a redundant selection of elements to update what was sent the server by update_status_for_marked_elements. Passing an elements argument instead allows more flexibility upstream and removes duplicated element selection code.

Binds arrow up and arrow down keys to increment_status_for_marked_elements in handle_keydown

@jzohrab
Copy link
Collaborator

jzohrab commented Dec 11, 2023

Looks great, thanks. I'll add a test in the dev branch.

@jzohrab jzohrab merged commit 365b09a into LuteOrg:develop Dec 11, 2023
9 of 10 checks passed
@jzohrab
Copy link
Collaborator

jzohrab commented Dec 11, 2023

Had to make one small change: setting the status to 0 isn't allowed, as that implies a missing term. The model isn't clear on this point, my spec was bad.

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