Skip to content

Conversation

@hannahbergam
Copy link
Contributor

@hannahbergam hannahbergam commented Jan 28, 2026

This wraps the fontAwesomeIcons in buttons, which makes them keyboard navigable. It also adds aria labels to each so a students knows they are toggling to see fish or trash. There are linter updates because I have prettier installed.

I tested via a symlink to the main repository- see the video below

Jira: https://codedotorg.atlassian.net/browse/SL-1548

Before: not much to video because the buttons weren't tab navigable at all

After:

Screen.Recording.2026-01-28.at.12.55.50.PM.mov

Followup: Make sure the student can then cycle through all the fish or trash floating in the pond to see how they did

@hannahbergam hannahbergam requested a review from a team February 3, 2026 00:15
@hannahbergam hannahbergam requested a review from a team February 4, 2026 21:02
Copy link

@fisher-alice fisher-alice left a comment

Choose a reason for hiding this comment

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

Nice! Left a couple non-blocking questions.

Copy link
Contributor

@bencodeorg bencodeorg left a comment

Choose a reason for hiding this comment

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

lgtm!

@@ -1,107 +1,109 @@
{
"fishvtrash-training-init1": "Garbage dumped in the water affects marine life. In this activity, you will program or train A.I. (artificial intelligence) to identify fish or trash. Let's clean up the ocean! Click anywhere on the screen to continue.",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Uuuugh prettier still fixing things in this repo- these are all just indents! Please review sans whitespace changes

@hannahbergam hannahbergam merged commit 1be9f15 into main Feb 10, 2026
1 of 3 checks passed
</button>
<button
type="button"
onClick={this.toggleRecall}

Choose a reason for hiding this comment

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

Sorry for late comment! Didn't notice this until I was able to pull latest branch and run locally.
I see that now that the toggle button is keyboard-navigable 🎉 and a user can select either the checkmark (matching) or banned (non-matching) buttons.
I am observing that when a user stays on one of the buttons, say the checkmark button and presses 'Enter' multiple times, the toggle action still happens, i.e., the matching fish set shows, then the non-matching fish set shows.
My wonder is maybe since the toggle button is now technically two separate buttons, would it make sense that if a user selects the checkmark, the matching fish set always shows and vice versa for the banned button.

toggle-button-current.mov

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason for hiding this comment

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

Here's a branch where I refactored the toggle function. I also removed some of the prior toggle button styling since we now have two buttons. main...alice/toggle-refactor

Screen.Recording.2026-02-10.at.12.13.44.PM.mov

Choose a reason for hiding this comment

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

Noting that the name of showRecallFish which was already here was a little confusing at first. I thought recallFish was the set of fish that was matching, but it's actually the set of fish that are NOT matching the given attribute! cc: @breville

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think pond vs recall could be something more like 'matching' and 'nonmatching' but I am fine either way

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants