Corina/644 fix a11y picks page #656
Conversation
|
Gridiron Survivor Application
Project name: Gridiron Survivor Application
Only deployments on the production branch are activated automatically. If you'd like to activate this deployment, navigate to your deployments. Learn more about Appwrite Function deployments.
|
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
api/apiFunctions.interface.ts
Outdated
| @@ -41,8 +41,11 @@ export interface IWeeklyPicks { | |||
| gameWeekId: string; | |||
| userResults: IUserPicksData; | |||
| } | |||
| // Currently, the pick history section on the | |||
| // Picks page uses this interface without requiring the teamId. Future | |||
| // accessibility fixes will most likely make use of the teamId. | |||
| export interface INFLTeam { | |||
There was a problem hiding this comment.
What else uses INFLTeam? Making this optional will cause Type issues down the long wrong. I'd suggest you keep this intact, create a new interface that extends INFLTeam and makes teamId required for the code that is using teamId as a requirement.
| @@ -43,7 +44,7 @@ import Heading from '@/components/Heading/Heading'; | |||
| */ | |||
| // eslint-disable-next-line no-unused-vars | |||
| const Week = ({ entry, league, NFLTeams, week }: IWeekProps): JSX.Element => { | |||
| const [pickHistory, setPickHistory] = useState<string[]>([]); | |||
| const [pickHistory, setPickHistory] = useState<INFLTeam[]>([]); | |||
| let entryHistory = currentEntry?.selectedTeams || []; | ||
| const entryHistory = currentEntry?.selectedTeams || []; | ||
|
|
||
| let entryHistoryObject: INFLTeam[] = []; |
There was a problem hiding this comment.
Looks like this is redundant because getPickHistory is only ran once. Therefore, the default value on line 47 should suffice. There's no need to resave an empty array if it's already the default value. It will cause unnecessary re-renders.
| @@ -160,15 +161,18 @@ const Week = ({ entry, league, NFLTeams, week }: IWeekProps): JSX.Element => { | |||
| } | |||
|
|
|||
| setEntryName(currentEntry.name); | |||
| let entryHistory = currentEntry?.selectedTeams || []; | |||
| const entryHistory = currentEntry?.selectedTeams || []; | |||
There was a problem hiding this comment.
Why did this function need to be refactored for this ticket?
There was a problem hiding this comment.
@shashilo
Previously, we only stored URLs for team logos in the pickHistory state. But we need to hold on to the team name as well to use it as the alt text of the image. That's why the entryHistoryObject.
| @@ -291,14 +295,14 @@ const Week = ({ entry, league, NFLTeams, week }: IWeekProps): JSX.Element => { | |||
| className="flex flex-wrap w-[90%] gap-4 overflow-x-scroll justify-center pb-10 items-center" | |||
| data-testid="user-pick-history" | |||
| > | |||
| {pickHistory?.map((logoURL, index) => { | |||
| {pickHistory?.map((team, index) => { | |||
There was a problem hiding this comment.
Why do we need to refactor this code for this ticket?
There was a problem hiding this comment.
@shashilo
The alt text for each logo used to be hardcoded as "team logo". We needed a way to assign it a dynamic value, ie the team's name.
| hasTeamBeenPicked(competition.team.name, selectedTeams) | ||
| } | ||
| isDisabled={false} | ||
| // isDisabled={ |
Accessibility issues on the Picks Page
Fixed with this PR:
altto the logos in the previous weeks sectionaltto the logo for each radio button (an alt text is redundant here since each button already has a label)htmlForattribute of each radio button's outer labelCloses #644
PS To test with a keyboard, press
tabto navigate through the interactive controls:tab(orShift+tab) will allow you to move from one radio group to the next/previous. In order to select the team, press theSpacebarSpacebar this time!). Because on this page selection triggers submission to the server, you should see a status message and will be sent to the All Entries page.