Parcourir la source

Remove incorrect "button" role from navigational links that are styled as buttons (#25504)

GitOrigin-RevId: 717b20a6f2e893034eb12547fa663d358c0de419
Rebeka Dekany il y a 1 an
Parent
commit
271635491a

+ 1 - 0
services/web/frontend/js/features/ui/components/bootstrap-5/button.tsx

@@ -52,6 +52,7 @@ const Button = forwardRef<HTMLButtonElement, ButtonProps>(
         ref={ref}
         disabled={isLoading || props.disabled}
         data-ol-loading={isLoading}
+        role={undefined}
       >
         {isLoading && (
           <span className="spinner-container">

+ 0 - 1
services/web/frontend/stylesheets/bootstrap-5/components/button.scss

@@ -252,7 +252,6 @@
 
 // Set the visited colour for a link that is styled as a button. This is necessary because we have a generic rule that
 // sets the colour of visited links
-a[role='button']:visited,
 a.btn:visited {
   color: var(--bs-btn-color);
 }

+ 4 - 4
services/web/test/frontend/features/group-management/components/add-seats.spec.tsx

@@ -31,7 +31,7 @@ describe('<AddSeats />', function () {
 
   it('renders the back button', function () {
     cy.findByTestId('group-heading').within(() => {
-      cy.findByRole('button', { name: /back to subscription/i }).should(
+      cy.findByRole('link', { name: /back to subscription/i }).should(
         'have.attr',
         'href',
         '/user/subscription'
@@ -71,7 +71,7 @@ describe('<AddSeats />', function () {
   })
 
   it('renders the cancel button', function () {
-    cy.findByRole('button', { name: /cancel/i }).should(
+    cy.findByRole('link', { name: /cancel/i }).should(
       'have.attr',
       'href',
       '/user/subscription'
@@ -213,7 +213,7 @@ describe('<AddSeats />', function () {
 
       describe('request', function () {
         afterEach(function () {
-          cy.findByRole('button', { name: /go to subscriptions/i }).should(
+          cy.findByRole('link', { name: /go to subscriptions/i }).should(
             'have.attr',
             'href',
             '/user/subscription'
@@ -414,7 +414,7 @@ describe('<AddSeats />', function () {
 
       describe('request', function () {
         afterEach(function () {
-          cy.findByRole('button', { name: /go to subscriptions/i }).should(
+          cy.findByRole('link', { name: /go to subscriptions/i }).should(
             'have.attr',
             'href',
             '/user/subscription'

+ 2 - 2
services/web/test/frontend/features/group-management/components/request-status.spec.tsx

@@ -12,7 +12,7 @@ describe('<RequestStatus />', function () {
 
   it('renders the back button', function () {
     cy.findByTestId('group-heading').within(() => {
-      cy.findByRole('button', { name: /back to subscription/i }).should(
+      cy.findByRole('link', { name: /back to subscription/i }).should(
         'have.attr',
         'href',
         '/user/subscription'
@@ -35,7 +35,7 @@ describe('<RequestStatus />', function () {
   })
 
   it('renders the link to subscriptions', function () {
-    cy.findByRole('button', { name: /go to subscriptions/i }).should(
+    cy.findByRole('link', { name: /go to subscriptions/i }).should(
       'have.attr',
       'href',
       '/user/subscription'

+ 1 - 1
services/web/test/frontend/features/group-management/components/upgrade-subscription.spec.tsx

@@ -66,7 +66,7 @@ describe('<UpgradeSubscription />', function () {
 
   it('shows the "Upgrade" and "Cancel" buttons', function () {
     cy.findByRole('button', { name: /upgrade/i })
-    cy.findByRole('button', { name: /cancel/i }).should(
+    cy.findByRole('link', { name: /cancel/i }).should(
       'have.attr',
       'href',
       '/user/subscription'

+ 1 - 1
services/web/test/frontend/features/project-list/components/current-plan-widget.test.tsx

@@ -70,7 +70,7 @@ describe('<CurrentPlanWidget />', function () {
     })
 
     it('clicks on upgrade button', function () {
-      const upgradeLink = screen.getByRole('button', { name: /upgrade/i })
+      const upgradeLink = screen.getByRole('link', { name: /upgrade/i })
       fireEvent.click(upgradeLink)
       expect(sendMBSpy).to.be.calledOnce
       expect(sendMBSpy).calledWith('upgrade-button-click', {

+ 13 - 13
services/web/test/frontend/features/project-list/components/notifications.test.tsx

@@ -141,7 +141,7 @@ describe('<UserNotifications />', function () {
 
       expect(screen.queryByRole('button', { name: /join project/i })).to.be.null
 
-      const openProject = screen.getByRole('button', { name: /open project/i })
+      const openProject = screen.getByRole('link', { name: /open project/i })
       expect(openProject.getAttribute('href')).to.equal(
         `/project/${notificationProjectInvite.messageOpts.projectId}`
       )
@@ -203,7 +203,7 @@ describe('<UserNotifications />', function () {
       screen.getByRole('alert')
       screen.getByText(/your free WFH2020 upgrade came to an end on/i)
 
-      const viewLink = screen.getByRole('button', { name: /view/i })
+      const viewLink = screen.getByRole('link', { name: /view/i })
       expect(viewLink.getAttribute('href')).to.equal(
         'https://www.overleaf.com/events/wfh2020'
       )
@@ -242,7 +242,7 @@ describe('<UserNotifications />', function () {
       expect(findOutMore.getAttribute('href')).to.equal(
         'https://www.overleaf.com/learn/how-to/Institutional_Login'
       )
-      const linkAccount = screen.getByRole('button', { name: /link account/i })
+      const linkAccount = screen.getByRole('link', { name: /link account/i })
       expect(linkAccount.getAttribute('href')).to.equal(
         `${exposedSettings.samlInitPath}?university_id=${notificationIPMatchedAffiliation.messageOpts.institutionId}&auto=/project`
       )
@@ -277,7 +277,7 @@ describe('<UserNotifications />', function () {
         /add an institutional email address to claim your features/i
       )
 
-      const addAffiliation = screen.getByRole('button', {
+      const addAffiliation = screen.getByRole('link', {
         name: /add affiliation/i,
       })
       expect(addAffiliation.getAttribute('href')).to.equal(`/user/settings`)
@@ -303,7 +303,7 @@ describe('<UserNotifications />', function () {
       screen.getByText(/file limit/i)
       screen.getByText(/You can't add more files to the project or sync it/i)
 
-      const accountSettings = screen.getByRole('button', {
+      const accountSettings = screen.getByRole('link', {
         name: /Open project/i,
       })
       expect(accountSettings.getAttribute('href')).to.equal('/project/123')
@@ -493,7 +493,7 @@ describe('<UserNotifications />', function () {
         '/learn/how-to/Institutional_Login'
       )
 
-      const action = screen.getByRole('button', { name: /link account/i })
+      const action = screen.getByRole('link', { name: /link account/i })
       expect(action.getAttribute('href')).to.equal(
         `${exposedSettings.samlInitPath}?university_id=${notificationsInstitution.institutionId}&auto=/project&email=${notificationsInstitution.email}`
       )
@@ -558,7 +558,7 @@ describe('<UserNotifications />', function () {
       screen.getByRole('alert')
       screen.getByText(/which is already registered with/i)
 
-      const action = screen.getByRole('button', { name: /find out more/i })
+      const action = screen.getByRole('link', { name: /find out more/i })
       expect(action.getAttribute('href')).to.equal(
         '/learn/how-to/Institutional_Login'
       )
@@ -931,7 +931,7 @@ describe('<UserNotifications />', function () {
       renderWithinProjectListProvider(GroupsAndEnterpriseBanner)
       await fetchMock.callHistory.flush(true)
 
-      expect(screen.queryByRole('button', { name: 'Contact Sales' })).to.be.null
+      expect(screen.queryByRole('link', { name: 'Contact Sales' })).to.be.null
     })
 
     it('shows the banner for users that have dismissed the previous banners', async function () {
@@ -941,7 +941,7 @@ describe('<UserNotifications />', function () {
       renderWithinProjectListProvider(GroupsAndEnterpriseBanner)
       await fetchMock.callHistory.flush(true)
 
-      await screen.findByRole('button', { name: 'Contact Sales' })
+      await screen.findByRole('link', { name: 'Contact Sales' })
     })
 
     it('shows the banner for users that have dismissed the banner more than 30 days ago', async function () {
@@ -956,7 +956,7 @@ describe('<UserNotifications />', function () {
       renderWithinProjectListProvider(GroupsAndEnterpriseBanner)
       await fetchMock.callHistory.flush(true)
 
-      await screen.findByRole('button', { name: 'Contact Sales' })
+      await screen.findByRole('link', { name: 'Contact Sales' })
     })
 
     it('does not show the banner for users that have dismissed the banner within the last 30 days', async function () {
@@ -971,7 +971,7 @@ describe('<UserNotifications />', function () {
       renderWithinProjectListProvider(GroupsAndEnterpriseBanner)
       await fetchMock.callHistory.flush(true)
 
-      expect(screen.queryByRole('button', { name: 'Contact Sales' })).to.be.null
+      expect(screen.queryByRole('link', { name: 'Contact Sales' })).to.be.null
     })
 
     describe('users that are not in group and are not affiliated', function () {
@@ -1012,7 +1012,7 @@ describe('<UserNotifications />', function () {
         await screen.findByText(
           'Overleaf On-Premises: Does your company want to keep its data within its firewall? Overleaf offers Server Pro, an on-premises solution for companies. Get in touch to learn more.'
         )
-        const link = screen.getByRole('button', { name: 'Contact Sales' })
+        const link = screen.getByRole('link', { name: 'Contact Sales' })
 
         expect(link.getAttribute('href')).to.equal(`/for/contact-sales-2`)
       })
@@ -1029,7 +1029,7 @@ describe('<UserNotifications />', function () {
         await screen.findByText(
           'Why do Fortune 500 companies and top research institutions trust Overleaf to streamline their collaboration? Get in touch to learn more.'
         )
-        const link = screen.getByRole('button', { name: 'Contact Sales' })
+        const link = screen.getByRole('link', { name: 'Contact Sales' })
 
         expect(link.getAttribute('href')).to.equal(`/for/contact-sales-4`)
       })

+ 1 - 1
services/web/test/frontend/features/project-list/components/sidebar/add-affiliation.test.tsx

@@ -31,7 +31,7 @@ describe('Add affiliation widget', function () {
     await waitFor(() => expect(fetchMock.callHistory.called('/api/project')))
 
     await screen.findByText(/are you affiliated with an institution/i)
-    const addAffiliationLink = screen.getByRole('button', {
+    const addAffiliationLink = screen.getByRole('link', {
       name: /add affiliation/i,
     })
     expect(addAffiliationLink.getAttribute('href')).to.equal('/user/settings')

+ 1 - 1
services/web/test/frontend/features/project-list/components/survey-widget.test.tsx

@@ -36,7 +36,7 @@ describe('<SurveyWidgetDsNav />', function () {
       screen.getByText(this.preText)
       screen.getByText(this.linkText)
 
-      const link = screen.getByRole('button', {
+      const link = screen.getByRole('link', {
         name: 'Take survey',
       }) as HTMLAnchorElement
       expect(link.href).to.equal(this.url)

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

@@ -69,7 +69,6 @@ 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/i })
 
@@ -81,7 +80,7 @@ describe('<LinkingSection />', function () {
       name: /learn more about orcid/i,
     })
     expect(helpLink.getAttribute('href')).to.equal('/blog/434')
-    const linkButton = screen.getByRole('button', { name: /link orcid/i })
+    const linkButton = screen.getByRole('link', { name: /link orcid/i })
     expect(linkButton.getAttribute('href')).to.equal('/auth/orcid?intent=link')
   })
 

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

@@ -33,7 +33,7 @@ describe('<IntegrationLinkingWidgetTest/>', function () {
     })
 
     it('should render an upgrade link and track clicks', function () {
-      const upgradeLink = screen.getByRole('button', { name: /upgrade/i })
+      const upgradeLink = screen.getByRole('link', { name: /upgrade/i })
       expect(upgradeLink.getAttribute('href')).to.equal(
         '/user/subscription/plans'
       )
@@ -52,7 +52,7 @@ describe('<IntegrationLinkingWidgetTest/>', function () {
 
     it('should render a link to initiate integration linking', function () {
       expect(
-        screen.getByRole('button', { name: 'Link' }).getAttribute('href')
+        screen.getByRole('link', { name: 'Link' }).getAttribute('href')
       ).to.equal('/link')
     })
 

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

@@ -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/i }).getAttribute('href')
+        screen.getByRole('link', { name: /link/i }).getAttribute('href')
       ).to.equal('/integration/link?intent=link')
     })
   })

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

@@ -22,7 +22,7 @@ describe('<SecuritySection />', function () {
     render(<SecuritySection />)
 
     expect(screen.getAllByText('Single Sign-On (SSO)').length).to.equal(2)
-    const link = screen.getByRole('button', {
+    const link = screen.getByRole('link', {
       name: /Set up SSO/i,
     })
     expect(link).to.exist

+ 1 - 1
services/web/test/frontend/features/subscription/components/dashboard/personal-subscription.test.tsx

@@ -82,7 +82,7 @@ describe('<PersonalSubscription />', function () {
 
       screen.getByText('No further payments will be taken.', { exact: false })
 
-      screen.getByRole('button', { name: 'View your invoices' })
+      screen.getByRole('link', { name: 'View your invoices' })
       screen.getByRole('button', { name: 'Reactivate your subscription' })
     })