From e9378bf73940504d8f62bb5f064903e149fbd506 Mon Sep 17 00:00:00 2001 From: Rebeka Dekany <50901361+rebekadekany@users.noreply.github.com> Date: Tue, 13 May 2025 12:02:47 +0200 Subject: [PATCH] Remove incorrect "button" role from navigational links that are styled as buttons (#25504) GitOrigin-RevId: 717b20a6f2e893034eb12547fa663d358c0de419 --- .../ui/components/bootstrap-5/button.tsx | 1 + .../bootstrap-5/components/button.scss | 1 - .../components/add-seats.spec.tsx | 8 +++--- .../components/request-status.spec.tsx | 4 +-- .../components/upgrade-subscription.spec.tsx | 2 +- .../components/current-plan-widget.test.tsx | 2 +- .../components/notifications.test.tsx | 26 +++++++++---------- .../sidebar/add-affiliation.test.tsx | 2 +- .../components/survey-widget.test.tsx | 2 +- .../components/linking-section.test.tsx | 3 +-- .../linking/integration-widget.test.tsx | 4 +-- .../components/linking/sso-widget.test.tsx | 2 +- .../components/security-section.test.tsx | 2 +- .../dashboard/personal-subscription.test.tsx | 2 +- 14 files changed, 30 insertions(+), 31 deletions(-) diff --git a/services/web/frontend/js/features/ui/components/bootstrap-5/button.tsx b/services/web/frontend/js/features/ui/components/bootstrap-5/button.tsx index 4eec23475a..2bbc97c695 100644 --- a/services/web/frontend/js/features/ui/components/bootstrap-5/button.tsx +++ b/services/web/frontend/js/features/ui/components/bootstrap-5/button.tsx @@ -52,6 +52,7 @@ const Button = forwardRef( ref={ref} disabled={isLoading || props.disabled} data-ol-loading={isLoading} + role={undefined} > {isLoading && ( diff --git a/services/web/frontend/stylesheets/bootstrap-5/components/button.scss b/services/web/frontend/stylesheets/bootstrap-5/components/button.scss index 3b782ed1f8..80220a6911 100644 --- a/services/web/frontend/stylesheets/bootstrap-5/components/button.scss +++ b/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); } diff --git a/services/web/test/frontend/features/group-management/components/add-seats.spec.tsx b/services/web/test/frontend/features/group-management/components/add-seats.spec.tsx index 211714db2b..ead0c74a1c 100644 --- a/services/web/test/frontend/features/group-management/components/add-seats.spec.tsx +++ b/services/web/test/frontend/features/group-management/components/add-seats.spec.tsx @@ -31,7 +31,7 @@ describe('', 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('', 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('', 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('', 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' diff --git a/services/web/test/frontend/features/group-management/components/request-status.spec.tsx b/services/web/test/frontend/features/group-management/components/request-status.spec.tsx index 79982a13d1..4a3061f464 100644 --- a/services/web/test/frontend/features/group-management/components/request-status.spec.tsx +++ b/services/web/test/frontend/features/group-management/components/request-status.spec.tsx @@ -12,7 +12,7 @@ describe('', 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('', 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' diff --git a/services/web/test/frontend/features/group-management/components/upgrade-subscription.spec.tsx b/services/web/test/frontend/features/group-management/components/upgrade-subscription.spec.tsx index 5918c1bd68..f7f2c564d7 100644 --- a/services/web/test/frontend/features/group-management/components/upgrade-subscription.spec.tsx +++ b/services/web/test/frontend/features/group-management/components/upgrade-subscription.spec.tsx @@ -66,7 +66,7 @@ describe('', 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' diff --git a/services/web/test/frontend/features/project-list/components/current-plan-widget.test.tsx b/services/web/test/frontend/features/project-list/components/current-plan-widget.test.tsx index 0ec6db3c9f..84836c204b 100644 --- a/services/web/test/frontend/features/project-list/components/current-plan-widget.test.tsx +++ b/services/web/test/frontend/features/project-list/components/current-plan-widget.test.tsx @@ -70,7 +70,7 @@ describe('', 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', { diff --git a/services/web/test/frontend/features/project-list/components/notifications.test.tsx b/services/web/test/frontend/features/project-list/components/notifications.test.tsx index f91399ff0c..db844f0260 100644 --- a/services/web/test/frontend/features/project-list/components/notifications.test.tsx +++ b/services/web/test/frontend/features/project-list/components/notifications.test.tsx @@ -141,7 +141,7 @@ describe('', 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('', 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('', 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('', 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('', 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('', 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('', 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('', 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('', 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('', 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('', 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('', 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('', 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`) }) diff --git a/services/web/test/frontend/features/project-list/components/sidebar/add-affiliation.test.tsx b/services/web/test/frontend/features/project-list/components/sidebar/add-affiliation.test.tsx index 8d69c2ddda..c49cd38f2c 100644 --- a/services/web/test/frontend/features/project-list/components/sidebar/add-affiliation.test.tsx +++ b/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') diff --git a/services/web/test/frontend/features/project-list/components/survey-widget.test.tsx b/services/web/test/frontend/features/project-list/components/survey-widget.test.tsx index bd6aeb683c..59f6bb9680 100644 --- a/services/web/test/frontend/features/project-list/components/survey-widget.test.tsx +++ b/services/web/test/frontend/features/project-list/components/survey-widget.test.tsx @@ -36,7 +36,7 @@ describe('', 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) diff --git a/services/web/test/frontend/features/settings/components/linking-section.test.tsx b/services/web/test/frontend/features/settings/components/linking-section.test.tsx index 887ada15de..056273ccc8 100644 --- a/services/web/test/frontend/features/settings/components/linking-section.test.tsx +++ b/services/web/test/frontend/features/settings/components/linking-section.test.tsx @@ -69,7 +69,6 @@ describe('', 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('', 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') }) diff --git a/services/web/test/frontend/features/settings/components/linking/integration-widget.test.tsx b/services/web/test/frontend/features/settings/components/linking/integration-widget.test.tsx index 435dbba0fa..1a623ba7b7 100644 --- a/services/web/test/frontend/features/settings/components/linking/integration-widget.test.tsx +++ b/services/web/test/frontend/features/settings/components/linking/integration-widget.test.tsx @@ -33,7 +33,7 @@ describe('', 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('', 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') }) diff --git a/services/web/test/frontend/features/settings/components/linking/sso-widget.test.tsx b/services/web/test/frontend/features/settings/components/linking/sso-widget.test.tsx index 5d29176c17..2f02b7709b 100644 --- a/services/web/test/frontend/features/settings/components/linking/sso-widget.test.tsx +++ b/services/web/test/frontend/features/settings/components/linking/sso-widget.test.tsx @@ -33,7 +33,7 @@ describe('', function () { it('should render a link to `linkPath`', function () { render() expect( - screen.getByRole('button', { name: /link/i }).getAttribute('href') + screen.getByRole('link', { name: /link/i }).getAttribute('href') ).to.equal('/integration/link?intent=link') }) }) diff --git a/services/web/test/frontend/features/settings/components/security-section.test.tsx b/services/web/test/frontend/features/settings/components/security-section.test.tsx index 08aa89e9a4..e1cfd98c2f 100644 --- a/services/web/test/frontend/features/settings/components/security-section.test.tsx +++ b/services/web/test/frontend/features/settings/components/security-section.test.tsx @@ -22,7 +22,7 @@ describe('', function () { render() 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 diff --git a/services/web/test/frontend/features/subscription/components/dashboard/personal-subscription.test.tsx b/services/web/test/frontend/features/subscription/components/dashboard/personal-subscription.test.tsx index 065aeec79f..6f09d66ab8 100644 --- a/services/web/test/frontend/features/subscription/components/dashboard/personal-subscription.test.tsx +++ b/services/web/test/frontend/features/subscription/components/dashboard/personal-subscription.test.tsx @@ -82,7 +82,7 @@ describe('', 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' }) })