Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: only show active router link styles on parent page #22069

Merged
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions packages/app/cypress/e2e/cypress-in-cypress-component.cy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -182,10 +182,10 @@ describe('Cypress In Cypress CT', { viewportWidth: 1500, defaultCommandTimeout:
cy.get('.passed > .num').should('contain', 1)
cy.get('.failed > .num').should('contain', '--')

cy.get('[href="#/runs"]').click()
cy.findByTestId('sidebar-link-runs-page').click()
cy.get('[data-cy="app-header-bar"]').findByText('Runs').should('be.visible')

cy.get('[href="#/specs"]').click()
cy.findByTestId('sidebar-link-specs-page').click()
cy.get('[data-cy="app-header-bar"]').findByText('Specs').should('be.visible')

cy.contains('TestComponent.spec').click()
Expand Down
4 changes: 2 additions & 2 deletions packages/app/cypress/e2e/cypress-in-cypress-e2e.cy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -233,10 +233,10 @@ describe('Cypress In Cypress E2E', { viewportWidth: 1500, defaultCommandTimeout:
cy.get('.passed > .num').should('contain', 1)
cy.get('.failed > .num').should('contain', '--')

cy.get('[href="#/runs"]').click()
cy.findByTestId('sidebar-link-runs-page').click()
cy.get('[data-cy="app-header-bar"]').findByText('Runs').should('be.visible')

cy.get('[href="#/specs"]').click()
cy.findByTestId('sidebar-link-specs-page').click()
cy.get('[data-cy="app-header-bar"]').findByText('Specs').should('be.visible')

cy.contains('dom-content.spec').click()
Expand Down
2 changes: 1 addition & 1 deletion packages/app/cypress/e2e/cypress-origin-communicator.cy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ describe('Cypress In Cypress Origin Communicator', () => {
// make sure duplicate logs are not present
cy.get('.command-name-origin').find('.command-name-log').should('have.length', 2)

cy.get('a[href="#/specs"]').click()
cy.findByTestId('sidebar-link-specs-page').click()
cy.location('hash').should('include', '/specs')

cy.contains('simple_origin.cy').click()
Expand Down
40 changes: 20 additions & 20 deletions packages/app/cypress/e2e/runs.cy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ function scaffoldTestingTypeAndVisitRunsPage (testingType: 'e2e' | 'component')

cy.visitApp()

return cy.get('[href="#/runs"]').click()
return cy.findByTestId('sidebar-link-runs-page').click()
}

describe('App: Runs', { viewportWidth: 1200 }, () => {
Expand Down Expand Up @@ -47,7 +47,7 @@ describe('App: Runs', { viewportWidth: 1200 }, () => {
})

cy.visitApp()
cy.get('[href="#/runs"]').click()
cy.findByTestId('sidebar-link-runs-page').click()
cy.get('[data-cy="runs-loader"]')
cy.get('[data-cy="runs"]')
})
Expand All @@ -62,13 +62,13 @@ describe('App: Runs', { viewportWidth: 1200 }, () => {

it('when logged out, shows call to action', () => {
cy.visitApp()
cy.get('[href="#/runs"]').click()
cy.findByTestId('sidebar-link-runs-page').click()
cy.contains(defaultMessages.runs.connect.buttonUser).should('exist')
})

it('clicking the login button will open the login modal', () => {
cy.visitApp()
cy.get('[href="#/runs"]').click()
cy.findByTestId('sidebar-link-runs-page').click()
cy.contains('Log In').click()
cy.findByRole('dialog', { name: 'Log in to Cypress' }).within(() => {
cy.get('button').contains('Log In')
Expand All @@ -79,7 +79,7 @@ describe('App: Runs', { viewportWidth: 1200 }, () => {
cy.loginUser()
cy.visitApp()

cy.get('[href="#/runs"]').click()
cy.findByTestId('sidebar-link-runs-page').click()
cy.contains('a', 'OVERLIMIT').click()

cy.withCtx((ctx) => {
Expand All @@ -105,7 +105,7 @@ describe('App: Runs', { viewportWidth: 1200 }, () => {
return obj.result
})

cy.get('[href="#/runs"]').click()
cy.findByTestId('sidebar-link-runs-page').click()

cy.findByText(defaultMessages.runs.connect.buttonProject).click()
cy.get('[aria-modal="true"]').should('exist')
Expand Down Expand Up @@ -145,7 +145,7 @@ describe('App: Runs', { viewportWidth: 1200 }, () => {
return obj.result
})

cy.get('[href="#/runs"]').click()
cy.findByTestId('sidebar-link-runs-page').click()

cy.findByText(defaultMessages.runs.connect.buttonProject).click()
cy.get('[aria-modal="true"]').should('exist')
Expand Down Expand Up @@ -175,7 +175,7 @@ describe('App: Runs', { viewportWidth: 1200 }, () => {
return obj.result
})

cy.get('[href="#/runs"]').click()
cy.findByTestId('sidebar-link-runs-page').click()
cy.findByText(defaultMessages.runs.connect.buttonProject).click()
cy.get('[aria-modal="true"]').should('exist')

Expand Down Expand Up @@ -217,7 +217,7 @@ describe('App: Runs', { viewportWidth: 1200 }, () => {
expect(config.projectId).to.not.equal('newProjectId')
})

cy.get('[href="#/runs"]').click()
cy.findByTestId('sidebar-link-runs-page').click()
cy.findByText(defaultMessages.runs.connect.buttonProject).click()
cy.get('button').contains(defaultMessages.runs.connect.modal.selectProject.createProject).click()
cy.findByText(defaultMessages.runs.connectSuccessAlert.title).should('be.visible')
Expand Down Expand Up @@ -263,7 +263,7 @@ describe('App: Runs', { viewportWidth: 1200 }, () => {

cy.visitApp()

cy.get('[href="#/runs"]').click()
cy.findByTestId('sidebar-link-runs-page').click()
})

it('if project Id is specified in config file that does not exist, shows call to action', () => {
Expand Down Expand Up @@ -389,7 +389,7 @@ describe('App: Runs', { viewportWidth: 1200 }, () => {

cy.visitApp()

cy.get('[href="#/runs"]').click()
cy.findByTestId('sidebar-link-runs-page').click()
})

it('if project Id is specified in config file that is not accessible, shows call to action', () => {
Expand All @@ -415,7 +415,7 @@ describe('App: Runs', { viewportWidth: 1200 }, () => {
})

cy.visitApp()
cy.get('[href="#/runs"]').click()
cy.findByTestId('sidebar-link-runs-page').click()
cy.findByText(defaultMessages.runs.connect.buttonProject).should('exist')
})

Expand Down Expand Up @@ -471,14 +471,14 @@ describe('App: Runs', { viewportWidth: 1200 }, () => {
it('displays a list of recorded runs if a run has been recorded', () => {
cy.loginUser()
cy.visitApp()
cy.get('[href="#/runs"]').click()
cy.findByTestId('sidebar-link-runs-page').click()
cy.get('[data-cy="runs"]')
})

it('displays each run with correct information', () => {
cy.loginUser()
cy.visitApp()
cy.get('[href="#/runs"]').click()
cy.findByTestId('sidebar-link-runs-page').click()

cy.get('[href="http://dummy.cypress.io/runs/0"]').first().within(() => {
cy.findByText('fix: make gql work CANCELLED')
Expand Down Expand Up @@ -515,7 +515,7 @@ describe('App: Runs', { viewportWidth: 1200 }, () => {
cy.loginUser()
cy.visitApp()

cy.get('[href="#/runs"]').click()
cy.findByTestId('sidebar-link-runs-page').click()
cy.get('[data-cy^="runCard-"]').first().click()

cy.withCtx((ctx) => {
Expand All @@ -540,7 +540,7 @@ describe('App: Runs', { viewportWidth: 1200 }, () => {
return obj.result
})

cy.get('[href="#/runs"]').click()
cy.findByTestId('sidebar-link-runs-page').click()
cy.contains('h2', 'Cannot connect to the Cypress Dashboard')
cy.percySnapshot()

Expand Down Expand Up @@ -571,7 +571,7 @@ describe('App: Runs', { viewportWidth: 1200 }, () => {
cy.loginUser()
cy.visitApp()
cy.wait(1000)
cy.get('[href="#/runs"]').click()
cy.findByTestId('sidebar-link-runs-page').click()
cy.get('[data-cy="runs"]')

cy.goOffline()
Expand All @@ -585,7 +585,7 @@ describe('App: Runs', { viewportWidth: 1200 }, () => {
cy.loginUser()
cy.visitApp()
cy.wait(1000)
cy.get('[href="#/runs"]').click()
cy.findByTestId('sidebar-link-runs-page').click()
cy.get('[data-cy="runs"]')

cy.goOffline()
Expand Down Expand Up @@ -696,10 +696,10 @@ describe('App: Runs', { viewportWidth: 1200 }, () => {
cy.get('[data-cy="run-card-icon-RUNNING"]').should('have.length', 2).should('be.visible')

// If we navigate away & back, we should see the same runs
cy.get('[href="#/settings"]').click()
cy.findByTestId('sidebar-link-settings-page').click()
cy.remoteGraphQLIntercept((obj) => obj.result)

cy.get('[href="#/runs"]').click()
cy.findByTestId('sidebar-link-runs-page').click()

cy.get('[data-cy="run-card-icon-PASSED"]').should('have.length', 3).should('be.visible')
cy.get('[data-cy="run-card-icon-RUNNING"]').should('have.length', 2).should('be.visible')
Expand Down
14 changes: 7 additions & 7 deletions packages/app/cypress/e2e/settings.cy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ describe('App: Settings', () => {
cy.waitForSpecToFinish()
// Wait for the test to pass, so the test is completed
cy.get('.passed > .num').should('contain', 1)
cy.get('[href="#/settings"]').click()
cy.findByTestId('sidebar-link-settings-page').click()
cy.contains('Dashboard Settings').click()
// Assert the data is not there before it arrives
cy.contains('Record Key').should('not.exist')
Expand All @@ -114,18 +114,18 @@ describe('App: Settings', () => {
o.sinon.spy(ctx.actions.auth, 'logout')
})

cy.get('[href="#/settings"]').click()
cy.findByTestId('sidebar-link-settings-page').click()
cy.contains('Dashboard Settings').click()
cy.contains('Record Key').should('exist')
cy.get('[href="#/runs"]').click()
cy.findByTestId('sidebar-link-runs-page').click()
cy.findByTestId('user-avatar-title').click()
cy.findByRole('button', { name: 'Log Out' }).click()

cy.withRetryableCtx((ctx, o) => {
expect(ctx.actions.auth.logout).to.have.been.calledOnce
})

cy.get('[href="#/settings"]').click()
cy.findByTestId('sidebar-link-settings-page').click()
cy.contains('Dashboard Settings').click()
cy.contains('Record Key').should('not.exist')
})
Expand Down Expand Up @@ -345,7 +345,7 @@ describe('App: Settings', () => {
// navigate away and come back
// preferred editor selected from dropdown should have been persisted
cy.visitApp()
cy.get('[href="#/settings"]').click()
cy.findByTestId('sidebar-link-settings-page').click()
cy.wait(200)
cy.get('[data-cy="Device Settings"]').click()

Expand All @@ -366,7 +366,7 @@ describe('App: Settings', () => {

// navigate away and come back
// preferred editor entered from input should have been persisted
cy.get('[href="#/settings"]').click()
cy.findByTestId('sidebar-link-settings-page').click()
cy.wait(100)
cy.get('[data-cy="Device Settings"]').click()

Expand Down Expand Up @@ -394,7 +394,7 @@ describe('App: Settings', () => {
// navigate away and come back
// preferred editor selected from dropdown should have been persisted
cy.visitApp()
cy.get('[href="#/settings"]').click()
cy.findByTestId('sidebar-link-settings-page').click()
cy.wait(200)
cy.get('[data-cy="Device Settings"]').click()

Expand Down
26 changes: 20 additions & 6 deletions packages/app/cypress/e2e/sidebar_navigation.cy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -137,17 +137,17 @@ describe('Sidebar Navigation', () => {
cy.percySnapshot()
cy.findByTestId('sidebar-header').trigger('mouseout')

cy.get('[href="#/runs"]').trigger('mouseenter')
cy.findByTestId('sidebar-link-runs-page').trigger('mouseenter')
cy.contains('.v-popper--some-open--tooltip', 'Runs')
cy.get('[href="#/runs"]').trigger('mouseout')
cy.findByTestId('sidebar-link-runs-page').trigger('mouseout')

cy.get('[href="#/specs"]').trigger('mouseenter')
cy.findByTestId('sidebar-link-specs-page').trigger('mouseenter')
cy.contains('.v-popper--some-open--tooltip', 'Specs')
cy.get('[href="#/specs"]').trigger('mouseout')
cy.findByTestId('sidebar-link-specs-page').trigger('mouseout')

cy.get('[href="#/settings"]').trigger('mouseenter')
cy.findByTestId('sidebar-link-settings-page').trigger('mouseenter')
cy.contains('.v-popper--some-open--tooltip', 'Settings')
cy.get('[href="#/settings"]').trigger('mouseout')
cy.findByTestId('sidebar-link-settings-page').trigger('mouseout')
})

it('opens the left nav bar when clicking the expand button (if unexpanded)', () => {
Expand Down Expand Up @@ -235,6 +235,20 @@ describe('Sidebar Navigation', () => {
cy.get('.router-link-active').findByText('Specs').should('be.visible')
})

it('Specs sidebar nav link is not active when a test is running', () => {
cy.location('hash').should('equal', '#/specs')
cy.contains('.router-link-exact-active', 'Specs')

cy.findAllByTestId('spec-item').first().click()
cy.location('hash').should('contain', '#/specs/runner')
cy.contains('.router-link-exact-active', 'Specs').should('not.exist')
cy.percySnapshot()

cy.get('a[href="#/specs"]').click()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does it make sense to use a more specific selector like cy.findAllByTestId('nav-link-spec') just in case another link was added later?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, good call!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The update looks elegant!

cy.location('hash').should('equal', '#/specs')
cy.contains('.router-link-exact-active', 'Specs')
})

it('has a menu item labeled "Settings" which takes you to the Settings page', () => {
cy.findByTestId('app-header-bar').findByText('Settings').should('not.exist')
cy.findByText('Settings').should('be.visible')
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ describe('App: Runs', { viewportWidth: 1200 }, () => {
return obj.result
})

cy.get('[href="#/runs"]').click()
cy.findByTestId('sidebar-link-runs-page').click()

cy.findByText(defaultMessages.runs.connect.buttonProject).click()
cy.get('[aria-modal="true"]').should('exist')
Expand Down
2 changes: 1 addition & 1 deletion packages/app/cypress/e2e/top-nav.cy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -382,7 +382,7 @@ describe('App Top Nav Workflows', () => {
cy.findByTestId('login-panel').contains('Test User').should('be.visible')
cy.findByTestId('login-panel').contains('test@example.com').should('be.visible')

cy.get('[href="#/runs"]').click()
cy.findByTestId('sidebar-link-runs-page').click()
cy.get('@logInButton').click()

cy.findByTestId('app-header-bar').within(() => {
Expand Down
9 changes: 7 additions & 2 deletions packages/app/src/navigation/SidebarNavigation.cy.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,9 @@ describe('SidebarNavigation', () => {
cy.contains('.v-popper--some-open--tooltip', 'test-project').should('be.visible')
cy.findByTestId('sidebar-header').trigger('mouseout')

cy.get('[href="#/runs"]').trigger('mouseenter')
cy.findByTestId('sidebar-link-runs-page').trigger('mouseenter')
cy.contains('.v-popper--some-open--tooltip', 'Runs').should('be.visible')
cy.get('[href="#/runs"]').trigger('mouseout')
cy.findByTestId('sidebar-link-runs-page').trigger('mouseout')
cy.percySnapshot()
})

Expand All @@ -55,4 +55,9 @@ describe('SidebarNavigation', () => {
cy.findByTestId('keyboard-modal-trigger').focus().type('{enter}')
cy.findByTestId('keyboard-modal').should('be.visible')
})

it('uses exact match for router link active class', () => {
mountComponent()
cy.get('[href="#/specs"]').should('have.class', 'router-link-exact-active')
})
})
5 changes: 3 additions & 2 deletions packages/app/src/navigation/SidebarNavigation.vue
Original file line number Diff line number Diff line change
Expand Up @@ -38,12 +38,13 @@
>
<RouterLink
v-for="item in navigation"
v-slot="{ isActive }"
v-slot="{ isExactActive }"
:key="item.name"
:to="item.href"
>
<SidebarNavigationRow
:active="isActive"
:data-cy="`sidebar-link-${item.name}-page`"
:active="isExactActive"
:icon="item.icon"
:name="item.name"
:is-nav-bar-expanded="isNavBarExpanded"
Expand Down