feat: add accessibility string for profile icon [WPB-21213] - #21831
feat: add accessibility string for profile icon [WPB-21213]#21831e-maad wants to merge 4 commits into
Conversation
|
screendriver
left a comment
screendriver
left a comment
There was a problem hiding this comment.
Please add tests for the newly constructed accessible name. At minimum, cover the availability status, combined MLS and Proteus verification, and the EXPIRES_SOON translation. The current expired/expiring-soon mistake would have been caught by such a test.
| hasPendingLegalHold, | ||
| } = useKoSubscribableChildren(user, ['hasPendingLegalHold', 'isOnLegalHold', 'name', 'username']); | ||
| } = useKoSubscribableChildren(user, ['availability', 'hasPendingLegalHold', 'isOnLegalHold', 'name', 'username']); | ||
| const verificationStatus = useUserVerificationStatus({user, groupId, isSelfUser: isTeam}); |
There was a problem hiding this comment.
This still causes useUserVerificationStatus to run twice. UserDetailsComponent calls it here, but the UserVerificationBadges rendered below calls the same hook again internally. The team-user path therefore still performs the identity lookup and registers the device-status listener twice.
Please calculate the verification status once and pass the resulting mlsStatus and isProteusVerified values to the rendered badges.
| }); | ||
| }); | ||
|
|
||
| describe('getUserVerificationBadgeLabel', () => { |
There was a problem hiding this comment.
These tests cover getUserVerificationBadgeLabel in isolation, but they do not test the newly constructed accessible name assigned to the button in UserDetails.
Please add component tests that render UserDetails and verify the button鈥檚 accessible name. At minimum, cover the availability status, combined MLS and Proteus verification, and the EXPIRES_SOON translation. Otherwise, the integration between the status values and the actual accessible name remains untested.
| <AvailabilityStateButtonWrapper | ||
| isTeam={isTeam} | ||
| ariaLabel={avatarAriaLabel} | ||
| showAvailabilityContextMenu={showAvailabilityContextMenu} |
There was a problem hiding this comment.
The other button that invokes showAvailabilityContextMenu still does not expose aria-haspopup="menu". Please add aria-haspopup="menu" to both buttons that open the availability menu.
| ): string | undefined => { | ||
| const labels: string[] = []; | ||
|
|
||
| match(mlsStatus) |
There was a problem hiding this comment.
getUserVerificationBadgeLabel has only one production caller, which passes the result of useUserVerificationStatus. That hook can only return MLSStatuses.VALID or undefined as mlsStatus.
The EXPIRED, EXPIRES_SOON, REVOKED, and NOT_ACTIVATED branches are therefore unreachable in production; they are exercised only by tests that manually pass values the real caller cannot provide.
Please limit this function and its tests to the states its actual caller can provide.
| const mlsStatus = | ||
| mlsStatuses && mlsStatuses.length > 0 && mlsStatuses.every(status => status === MLSStatuses.VALID) | ||
| ? MLSStatuses.VALID | ||
| : undefined; |
| export const getUserVerificationBadgeLabel = ( | ||
| translate: RootContextValue['translate'], | ||
| {mlsStatus, isProteusVerified}: {mlsStatus?: MLSStatuses; isProteusVerified?: boolean}, | ||
| ): string | undefined => { |
There was a problem hiding this comment.
Can we avoid undefined and use Maybe instead?
2 participants
Description
Add accessibility string for profile link