Переглянути джерело

Improve ARIA labels for buttons and links on the Account setting page (#25499)

* Improve announced button and link labels for screen reader users

* Improve labels for integration widgets and update test

* Make integration widget IDs to be required

GitOrigin-RevId: 1e0124ef63a91fb63dffd79881c60794bccb9d27
Rebeka Dekany 1 рік тому
батько
коміт
ac51878186

+ 2 - 2
server-ce/test/git-bridge.spec.ts

@@ -93,7 +93,7 @@ describe('git-bridge', function () {
           cy.get('code').contains(`git clone ${gitURL(id.toString())}`)
         })
         cy.findByRole('button', {
-          name: 'Generate token',
+          name: /generate token/i,
         }).click()
         cy.get('code').contains(/olp_[a-zA-Z0-9]{16}/)
       })
@@ -196,7 +196,7 @@ describe('git-bridge', function () {
           cy.get('code').contains(`git clone ${gitURL(projectId.toString())}`)
         })
         cy.findByRole('button', {
-          name: 'Generate token',
+          name: /generate token/i,
         }).click()
         cy.get('code')
           .contains(/olp_[a-zA-Z0-9]{16}/)

+ 3 - 0
services/web/frontend/extracted-translations.json

@@ -895,10 +895,12 @@
   "layout_options": "",
   "layout_processing": "",
   "learn_more": "",
+  "learn_more_about": "",
   "learn_more_about_account": "",
   "learn_more_about_compile_timeouts": "",
   "learn_more_about_link_sharing": "",
   "learn_more_about_managed_users": "",
+  "learn_more_about_managing_email": "",
   "learn_more_about_other_causes_of_compile_timeouts": "",
   "leave": "",
   "leave_any_group_subscriptions": "",
@@ -1144,6 +1146,7 @@
   "other_causes_of_compile_timeouts": "",
   "other_logs_and_files": "",
   "other_output_files": "",
+  "our_help_page": "",
   "our_team_will_get_back_to_you_shortly": "",
   "our_values": "",
   "out_of_sync": "",

+ 2 - 1
services/web/frontend/js/features/settings/components/account-info-section.tsx

@@ -70,7 +70,7 @@ function AccountInfoSection() {
 
   return (
     <>
-      <h3>{t('update_account_info')}</h3>
+      <h3 id="update-account-info">{t('update_account_info')}</h3>
       <form id="account-info-form" onSubmit={handleSubmit}>
         {hasAffiliationsFeature ? null : (
           <ReadOrWriteFormGroup
@@ -128,6 +128,7 @@ function AccountInfoSection() {
               disabled={!isFormValid}
               isLoading={isLoading}
               loadingLabel={t('saving') + '…'}
+              aria-labelledby={isLoading ? undefined : 'update-account-info'}
             >
               {t('update')}
             </OLButton>

+ 1 - 0
services/web/frontend/js/features/settings/components/emails-section.tsx

@@ -40,6 +40,7 @@ function EmailsSectionContent() {
             <a
               href="/learn/how-to/Managing_your_Overleaf_emails"
               target="_blank"
+              aria-label={t('learn_more_about_managing_email')}
             />,
           ]}
         />

+ 18 - 3
services/web/frontend/js/features/settings/components/linking/integration-widget.tsx

@@ -20,6 +20,7 @@ function trackLinkingClick(integration: string) {
 }
 
 type IntegrationLinkingWidgetProps = {
+  id: string
   logo: ReactNode
   title: string
   description: string
@@ -35,6 +36,7 @@ type IntegrationLinkingWidgetProps = {
 }
 
 export function IntegrationLinkingWidget({
+  id,
   logo,
   title,
   description,
@@ -65,12 +67,17 @@ export function IntegrationLinkingWidget({
       <div>{logo}</div>
       <div className="description-container">
         <div className="title-row">
-          <h4>{title}</h4>
+          <h4 id={id}>{title}</h4>
           {!hasFeature && <OLBadge bg="info">{t('premium_feature')}</OLBadge>}
         </div>
         <p className="small">
           {description}{' '}
-          <a href={helpPath} target="_blank" rel="noreferrer">
+          <a
+            href={helpPath}
+            target="_blank"
+            rel="noreferrer"
+            aria-label={t('learn_more_about', { integrationName: title })}
+          >
             {t('learn_more')}
           </a>
         </p>
@@ -78,6 +85,7 @@ export function IntegrationLinkingWidget({
       </div>
       <div>
         <ActionButton
+          titleId={id}
           integration={title}
           hasFeature={hasFeature}
           linked={linked}
@@ -105,6 +113,7 @@ type ActionButtonProps = {
   handleUnlinkClick: () => void
   linkPath: string
   disabled?: boolean
+  titleId: string
 }
 
 function ActionButton({
@@ -114,16 +123,22 @@ function ActionButton({
   linkPath,
   disabled,
   integration,
+  titleId,
 }: ActionButtonProps) {
   const { t } = useTranslation()
+  const linkTextId = `${titleId}-link`
+
   if (!hasFeature) {
     return (
       <OLButton
         variant="primary"
         href="/user/subscription/plans"
         onClick={() => trackUpgradeClick(integration)}
+        aria-labelledby={`${titleId} ${linkTextId}`}
       >
-        <span className="text-capitalize">{t('upgrade')}</span>
+        <span id={linkTextId} className="text-capitalize">
+          {t('upgrade')}
+        </span>
       </OLButton>
     )
   } else if (linked) {

+ 25 - 4
services/web/frontend/js/features/settings/components/linking/sso-widget.tsx

@@ -69,12 +69,17 @@ export function SSOLinkingWidget({
       <div>{providerLogos[providerId]}</div>
       <div className="description-container">
         <div className="title-row">
-          <h4>{title}</h4>
+          <h4 id={providerId}>{title}</h4>
         </div>
         <p className="small">
           {description?.replace(/<[^>]+>/g, '')}{' '}
           {helpPath ? (
-            <a href={helpPath} target="_blank" rel="noreferrer">
+            <a
+              href={helpPath}
+              target="_blank"
+              rel="noreferrer"
+              aria-label={t('learn_more_about', { integrationName: title })}
+            >
               {t('learn_more')}
             </a>
           ) : null}
@@ -85,6 +90,7 @@ export function SSOLinkingWidget({
       </div>
       <div>
         <ActionButton
+          titleId={providerId}
           unlinkRequestInflight={unlinkRequestInflight}
           accountIsLinked={linked}
           linkPath={`${linkPath}?intent=link`}
@@ -106,6 +112,7 @@ type ActionButtonProps = {
   accountIsLinked?: boolean
   linkPath: string
   onUnlinkClick: () => void
+  titleId: string
 }
 
 function ActionButton({
@@ -113,8 +120,11 @@ function ActionButton({
   accountIsLinked,
   linkPath,
   onUnlinkClick,
+  titleId,
 }: ActionButtonProps) {
   const { t } = useTranslation()
+  const linkTextId = `${titleId}-link`
+
   if (unlinkRequestInflight) {
     return (
       <OLButton variant="danger-ghost" disabled>
@@ -123,13 +133,24 @@ function ActionButton({
     )
   } else if (accountIsLinked) {
     return (
-      <OLButton variant="danger-ghost" onClick={onUnlinkClick}>
+      <OLButton
+        variant="danger-ghost"
+        onClick={onUnlinkClick}
+        aria-labelledby={`${linkTextId} ${titleId}`}
+        id={linkTextId}
+      >
         {t('unlink')}
       </OLButton>
     )
   } else {
     return (
-      <OLButton variant="secondary" href={linkPath} className="text-capitalize">
+      <OLButton
+        variant="secondary"
+        href={linkPath}
+        className="text-capitalize"
+        aria-labelledby={`${linkTextId} ${titleId}`}
+        id={linkTextId}
+      >
         {t('link')}
       </OLButton>
     )

+ 3 - 0
services/web/locales/en.json

@@ -1174,11 +1174,13 @@
   "ldap_create_admin_instructions": "Choose an email address for the first __appName__ admin account. This should correspond to an account in the LDAP system. You will then be asked to log in with this account.",
   "learn": "Learn",
   "learn_more": "Learn more",
+  "learn_more_about": "Learn more about __integrationName__",
   "learn_more_about_account": "<0>Learn more</0> about managing your __appName__ account.",
   "learn_more_about_compile_timeouts": "<0>Learn more</0> about compile timeouts.",
   "learn_more_about_emails": "<0>Learn more</0> about managing your __appName__ emails.",
   "learn_more_about_link_sharing": "Learn more about Link Sharing",
   "learn_more_about_managed_users": "Learn more about Managed Users.",
+  "learn_more_about_managing_email": "Learn more about managing your __appName__ emails.",
   "learn_more_about_other_causes_of_compile_timeouts": "<0>Learn more</0> about other causes of compile timeouts and how to fix them.",
   "leave": "Leave",
   "leave_any_group_subscriptions": "Leave any group subscriptions other than the one that will be managing your account. <0>Leave them from the Subscription page.</0>",
@@ -1511,6 +1513,7 @@
   "other_output_files": "Download other output files",
   "other_sessions": "Other Sessions",
   "other_ways_to_log_in": "Other ways to log in",
+  "our_help_page": "Our help page",
   "our_team_will_get_back_to_you_shortly": "Our team will get back to you shortly.",
   "our_values": "Our values",
   "out_of_sync": "Out of sync",

+ 10 - 10
services/web/test/frontend/features/settings/components/account-info-section.test.tsx

@@ -47,7 +47,7 @@ describe('<AccountInfoSection />', function () {
     })
     fireEvent.click(
       screen.getByRole('button', {
-        name: 'Update',
+        name: /update/i,
       })
     )
     expect(updateMock.callHistory.called()).to.be.true
@@ -68,7 +68,7 @@ describe('<AccountInfoSection />', function () {
       target: { value: 'john' },
     })
     const button = screen.getByRole('button', {
-      name: 'Update',
+      name: /update/i,
     }) as HTMLButtonElement
 
     expect(button.disabled).to.be.true
@@ -87,14 +87,14 @@ describe('<AccountInfoSection />', function () {
 
     fireEvent.click(
       screen.getByRole('button', {
-        name: 'Update',
+        name: /update/i,
       })
     )
     await screen.findByRole('button', { name: /saving/i })
 
     finishUpdateCall(200)
     await screen.findByRole('button', {
-      name: 'Update',
+      name: /update/i,
     })
     screen.getByText('Thanks, your settings have been updated.')
   })
@@ -105,7 +105,7 @@ describe('<AccountInfoSection />', function () {
 
     fireEvent.click(
       screen.getByRole('button', {
-        name: 'Update',
+        name: /update/i,
       })
     )
     await screen.findByText('Something went wrong. Please try again.')
@@ -117,7 +117,7 @@ describe('<AccountInfoSection />', function () {
 
     fireEvent.click(
       screen.getByRole('button', {
-        name: 'Update',
+        name: /update/i,
       })
     )
     await screen.findByText(
@@ -136,7 +136,7 @@ describe('<AccountInfoSection />', function () {
 
     fireEvent.click(
       screen.getByRole('button', {
-        name: 'Update',
+        name: /update/i,
       })
     )
     await screen.findByText('This email is already registered')
@@ -153,7 +153,7 @@ describe('<AccountInfoSection />', function () {
 
     fireEvent.click(
       screen.getByRole('button', {
-        name: 'Update',
+        name: /update/i,
       })
     )
     expect(
@@ -184,7 +184,7 @@ describe('<AccountInfoSection />', function () {
 
     fireEvent.click(
       screen.getByRole('button', {
-        name: 'Update',
+        name: /update account info/i,
       })
     )
     expect(
@@ -212,7 +212,7 @@ describe('<AccountInfoSection />', function () {
 
     fireEvent.click(
       screen.getByRole('button', {
-        name: 'Update',
+        name: /update/i,
       })
     )
     expect(

+ 19 - 10
services/web/test/frontend/features/settings/components/emails/emails-section-add-new-email.test.tsx

@@ -106,7 +106,7 @@ describe('<EmailsSection />', function () {
     })
     fireEvent.click(button)
 
-    await screen.findByLabelText(/email/i)
+    await screen.findByLabelText(/email/i, { selector: 'input' })
   })
 
   it('renders "Start adding your address" until a valid email is typed', async function () {
@@ -121,7 +121,7 @@ describe('<EmailsSection />', function () {
     })
     fireEvent.click(button)
 
-    const input = screen.getByLabelText(/email/i)
+    const input = screen.getByLabelText(/email/i, { selector: 'input' })
 
     // initially the text is displayed and the "add email" button disabled
     screen.getByText('Start by adding your email address.')
@@ -200,7 +200,7 @@ describe('<EmailsSection />', function () {
       .post('/user/emails/confirm-secondary', 200)
 
     fireEvent.click(addAnotherEmailBtn)
-    const input = screen.getByLabelText(/email/i)
+    const input = screen.getByLabelText(/email/i, { selector: 'input' })
 
     fireEvent.change(input, {
       target: { value: userEmailData.email },
@@ -242,7 +242,7 @@ describe('<EmailsSection />', function () {
       .post('/user/emails/secondary', 400)
 
     fireEvent.click(addAnotherEmailBtn)
-    const input = screen.getByLabelText(/email/i)
+    const input = screen.getByLabelText(/email/i, { selector: 'input' })
 
     fireEvent.change(input, {
       target: { value: userEmailData.email },
@@ -279,7 +279,7 @@ describe('<EmailsSection />', function () {
 
     await userEvent.click(button)
 
-    const input = screen.getByLabelText(/email/i)
+    const input = screen.getByLabelText(/email/i, { selector: 'input' })
     fireEvent.change(input, {
       target: { value: 'user@autocomplete.edu' },
     })
@@ -302,7 +302,10 @@ describe('<EmailsSection />', function () {
 
     await userEvent.click(button)
 
-    await userEvent.type(screen.getByLabelText(/email/i), userEmailData.email)
+    await userEvent.type(
+      screen.getByLabelText(/email/i, { selector: 'input' }),
+      userEmailData.email
+    )
 
     await userEvent.click(screen.getByRole('button', { name: /let us know/i }))
 
@@ -415,7 +418,10 @@ describe('<EmailsSection />', function () {
 
     // open "add new email" section and click "let us know" to open the Country/University form
     await userEvent.click(button)
-    await userEvent.type(screen.getByLabelText(/email/i), userEmailData.email)
+    await userEvent.type(
+      screen.getByLabelText(/email/i, { selector: 'input' }),
+      userEmailData.email
+    )
     await userEvent.click(screen.getByRole('button', { name: /let us know/i }))
 
     // select a country
@@ -457,7 +463,10 @@ describe('<EmailsSection />', function () {
 
     await userEvent.click(button)
 
-    await userEvent.type(screen.getByLabelText(/email/i), userEmailData.email)
+    await userEvent.type(
+      screen.getByLabelText(/email/i, { selector: 'input' }),
+      userEmailData.email
+    )
 
     await userEvent.click(screen.getByRole('button', { name: /let us know/i }))
 
@@ -574,7 +583,7 @@ describe('<EmailsSection />', function () {
     await userEvent.click(button)
 
     await userEvent.type(
-      screen.getByLabelText(/email/i),
+      screen.getByLabelText(/email/i, { selector: 'input' }),
       `user@${hostnameFirstChar}`
     )
 
@@ -647,7 +656,7 @@ describe('<EmailsSection />', function () {
     await userEvent.click(button)
 
     await userEvent.type(
-      screen.getByLabelText(/email/i),
+      screen.getByLabelText(/email/i, { selector: 'input' }),
       `user@${hostnameFirstChar}`
     )
 

+ 1 - 0
services/web/test/frontend/features/settings/components/emails/emails-section.test.tsx

@@ -40,6 +40,7 @@ describe('<EmailsSection />', function () {
 
     screen.getByText(/add additional email addresses/i)
     screen.getByText(/to change your primary email/i)
+    screen.getByLabelText('Learn more about managing your Overleaf emails.')
   })
 
   it('renders a loading message when loading', async function () {

+ 6 - 3
services/web/test/frontend/features/settings/components/linking-section.test.tsx

@@ -69,16 +69,19 @@ describe('<LinkingSection />', function () {
     screen.getByText('linked accounts')
 
     screen.getByText('Google')
+    screen.getByRole('button', { name: /link google/i })
     screen.getByText('Log in with Google.')
-    screen.getByRole('button', { name: 'Unlink' })
+    screen.getByRole('button', { name: /unlink/i })
 
     screen.getByText('ORCID')
     screen.getByText(
       /Securely establish your identity by linking your ORCID iD/
     )
-    const helpLink = screen.getByRole('link', { name: 'Learn more' })
+    const helpLink = screen.getByRole('link', {
+      name: /learn more about orcid/i,
+    })
     expect(helpLink.getAttribute('href')).to.equal('/blog/434')
-    const linkButton = screen.getByRole('button', { name: 'Link' })
+    const linkButton = screen.getByRole('button', { name: /link orcid/i })
     expect(linkButton.getAttribute('href')).to.equal('/auth/orcid?intent=link')
   })
 

+ 2 - 1
services/web/test/frontend/features/settings/components/linking/integration-widget.test.tsx

@@ -6,6 +6,7 @@ import * as eventTracking from '@/infrastructure/event-tracking'
 
 describe('<IntegrationLinkingWidgetTest/>', function () {
   const defaultProps = {
+    id: 'integration-widget-id',
     logo: <div />,
     title: 'Integration',
     description: 'paragraph1',
@@ -32,7 +33,7 @@ describe('<IntegrationLinkingWidgetTest/>', function () {
     })
 
     it('should render an upgrade link and track clicks', function () {
-      const upgradeLink = screen.getByRole('button', { name: 'Upgrade' })
+      const upgradeLink = screen.getByRole('button', { name: /upgrade/i })
       expect(upgradeLink.getAttribute('href')).to.equal(
         '/user/subscription/plans'
       )

+ 10 - 10
services/web/test/frontend/features/settings/components/linking/sso-widget.test.tsx

@@ -25,7 +25,7 @@ describe('<SSOLinkingWidget />', function () {
     screen.getByText('integration')
     screen.getByText('integration description')
     expect(
-      screen.getByRole('link', { name: 'Learn more' }).getAttribute('href')
+      screen.getByRole('link', { name: /learn more/i }).getAttribute('href')
     ).to.equal('/help/integration')
   })
 
@@ -33,7 +33,7 @@ describe('<SSOLinkingWidget />', function () {
     it('should render a link to `linkPath`', function () {
       render(<SSOLinkingWidget {...defaultProps} linked={false} />)
       expect(
-        screen.getByRole('button', { name: 'Link' }).getAttribute('href')
+        screen.getByRole('button', { name: /link/i }).getAttribute('href')
       ).to.equal('/integration/link?intent=link')
     })
   })
@@ -49,11 +49,11 @@ describe('<SSOLinkingWidget />', function () {
     })
 
     it('should display an `unlink` button', function () {
-      screen.getByRole('button', { name: 'Unlink' })
+      screen.getByRole('button', { name: /unlink/i })
     })
 
     it('should open a modal to confirm integration unlinking', function () {
-      fireEvent.click(screen.getByRole('button', { name: 'Unlink' }))
+      fireEvent.click(screen.getByRole('button', { name: /unlink/i }))
       screen.getByText('Unlink integration Account')
       screen.getByText(
         'Warning: When you unlink your account from integration you will not be able to sign in using integration anymore.'
@@ -61,7 +61,7 @@ describe('<SSOLinkingWidget />', function () {
     })
 
     it('should cancel unlinking when clicking cancel in the confirmation modal', async function () {
-      fireEvent.click(screen.getByRole('button', { name: 'Unlink' }))
+      fireEvent.click(screen.getByRole('button', { name: /unlink/i }))
       const cancelBtn = screen.getByRole('button', {
         name: 'Cancel',
         hidden: false,
@@ -80,9 +80,9 @@ describe('<SSOLinkingWidget />', function () {
       render(
         <SSOLinkingWidget {...defaultProps} linked onUnlink={unlinkFunction} />
       )
-      fireEvent.click(screen.getByRole('button', { name: 'Unlink' }))
+      fireEvent.click(screen.getByRole('button', { name: /unlink/i }))
       confirmBtn = within(screen.getByRole('dialog')).getByRole('button', {
-        name: 'Unlink',
+        name: /unlink/i,
         hidden: false,
       })
     })
@@ -114,11 +114,11 @@ describe('<SSOLinkingWidget />', function () {
       render(
         <SSOLinkingWidget {...defaultProps} linked onUnlink={unlinkFunction} />
       )
-      fireEvent.click(screen.getByRole('button', { name: 'Unlink' }))
+      fireEvent.click(screen.getByRole('button', { name: /unlink/i }))
       const confirmBtn = within(screen.getByRole('dialog')).getByRole(
         'button',
         {
-          name: 'Unlink',
+          name: /unlink/i,
           hidden: false,
         }
       )
@@ -130,7 +130,7 @@ describe('<SSOLinkingWidget />', function () {
     })
 
     it('should display the unlink button ', async function () {
-      await screen.findByRole('button', { name: 'Unlink' })
+      await screen.findByRole('button', { name: /unlink/i })
     })
   })
 })