Accessibility edits

[DynieM]

---

• To see the specific tasks where the Asana app for GitHub is being used, see below:
  • https://app.asana.com/0/0/1207543795708283

[clpetersonucf]

I appreciate the new PR that supersedes #38 and includes changes from a branch other than master. However, it looks like your last commit to this branch might have caused some unintended issues - for example, the function definitions for both assistiveNotification and _assistiveAlert were removed, which is causing console errors and breaks the player.

[dmols]

Looks really good! Tabbing works as expected and VO callouts are great. The only thing I'd recommend adding is a note of how many astronauts are left to sort, after a pairing has been made. If it's not a pain to implement, it could also be useful to note after shifting pages, so the user has a quick idea of how many pairings they have left to make.

[cayb0rg]

I like the new addition! Just one bug I noticed: it seems that the screenreader dialog for how many matches are completed is carried over into the current page when focusing the previous page arrow. (EDIT: this might have been a browser issue as I am no longer experiencing it on new laptop; however, when there are no matches complete, the dialog will say "of 5 complete" instead of "0 of 5 complete" or similar)

Edit: one other thing - everything but the Close button in the instruction manual has cursor: pointer on hover

[cayb0rg]

This function appears to be causing the "___ of 5 remaining" issue. It resets the completePerPage array before refilling it using the matches each time a match is made. It would need another loop that finds pages with no matches, and sets those pages' counts to 0 in the completePerPage array.

The more efficient method may be to just replace the call to _updateCompletionStatus with $scope.completePerPage[$scope.currentPage]++ inside _pushMatch.

[DynieM]

Thank you!

[cayb0rg]

Nice work! I believe I made an oversight when proposing the above solution to the "blank of 5" problem, sorry. I did not realize _pushMatch is called every time a match is made even if that is using an answer/question that's already matched to something. Doing so will cause results like these:

So, we probably do need to iterate through the $scope.matches array, or any other method as you see fit. Note that this still needs to account for pages with 0 matches completed so that "[blank] of 5" doesn't happen.

I'm also seeing an issue when a match is made multiple times using the same question/answer and the matches left is wrong. Upon a cursory glance, I'm guessing this has something to do with how the number of matches left is calculated on lines 272-274 of player.coffee.

Also, I noticed there are no instructions for shortcut keys (such as using 1,2,3,4,5 for left column and Q,W,E,R,T,Y, etc. for right column) for keyboard users.

[clpetersonucf]

This looks and feels great. Really nice work.

Only one piece of feedback before I stamp it with an approval: the .vscode/launch.json file and corresponding directory shouldn't be included in the git history, since it appears to be used for debugging and doesn't really belong in the repo. Once you remove it, this is good to go.

[clpetersonucf]

All requested changes are in. Looks good. 👍

[cayb0rg]

LGTM! Awesome work.

[clpetersonucf]

Unfortunately the player test failures uncovered some issues that went unnoticed previously. You'll have to correct them before LCC is good to go.

Also note that I pushed some changes to your branch to bring it up to parity with the target branch, dev/2.1.0, so make sure you git pull from the remote before doing anything. LCC is now using MWDK 3.x, so you'll also have to do the following after pulling the changes:

1. rm node_modules to remove the current node modules installed for the widget
2. Use nvm to install and switch to node 18.13.0 instead of 12.3.1 via nvm install 18.13.0 and nvm use 18.13.0
3. yarn install to get the new node packages

After which you can run yarn start and continue normally.

[clpetersonucf]

These lines were not causing any sort of console of behavioral failure, so they got overlooked when I reviewed your changes previously, but FYI this is not how you assign attributes to DOM elements in JS. Even though helpModal is referencing a DOM element object, attributes must be set (or removed) via the setAttribute or removeAttribute methods, like so:

helpModal.setAttribute('inert', '')

Bear in mind that inert doesn't require a value, so the mere existence of the attribute means the inert behavior will be applied. Therefore, helpModal.setAttribute('inert', 'false') wouldn't work to disable it. The attribute has to be removed completely with removeAttribute. This change will need to be applied on all lines that are trying to set or remove the inert attribute.

[clpetersonucf]

This line is still improperly trying to set the inert property on the DOM object directly, instead of using setAttribute(). It's causing the test suite to fail. Always remember to run yarn test before committing!

[clpetersonucf]

Still one reference to inert that you didn't update, once that's corrected, I think it's good.

[clpetersonucf]

This line is still improperly trying to set the inert property on the DOM object directly, instead of using setAttribute(). It's causing the test suite to fail. Always remember to run yarn test before committing!

[clpetersonucf]

Ready to go.