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.

Sign up for GitHub

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

Jump to bottom

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

Merged
merged 8 commits into from
Jun 3, 2022
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
88f6e38
use isExactActive
rachelruderman Jun 1, 2022
abc2817
add tests
rachelruderman Jun 2, 2022
da29d38
Merge branch 'develop' into 21873/indicate-how-to-get-to-specs-from-r…
rachelruderman Jun 2, 2022
4327281
add testId for sidebar links
rachelruderman Jun 3, 2022
6c222ee
Use testId II
rachelruderman Jun 3, 2022
4c815fd
Use testId III
rachelruderman Jun 3, 2022
6945a3d
Merge branch 'develop' into 21873/indicate-how-to-get-to-specs-from-r…
rachelruderman Jun 3, 2022
934591e
move testId
rachelruderman Jun 3, 2022
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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.findByTestId('sidebar-link-specs-page').click()
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
2 changes: 1 addition & 1 deletion packages/app/cypress/e2e/subscriptions/createCloudOrgModal-subscription.cy.ts
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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.findByTestId('sidebar-link-specs-page').should('have.class', 'router-link-exact-active')
})
})
5 changes: 3 additions & 2 deletions packages/app/src/navigation/SidebarNavigation.vue
View file
Open in desktop
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"
:data-cy="`sidebar-link-${item.name.toLowerCase()}-page`"
>
<SidebarNavigationRow
:active="isActive"
:active="isExactActive"
Comment on lines -46 to +47
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I opted for a global change instead of a Specs-only exception to ensure all navigation links continue to have the same behavior.

This means that if any of the other pages have links to a child view with a different URL (whether it be query params, pathname addition, etc) the navigation link will lose its active styles. This is intentional so that the user can more intuitively understand how to return to the parent page.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah I agree with doing this as the default. It's likely in the future, if we have nested router views, there would be cases where the exact route changes but the user is still "on" the main page. But I'm fine with not worrying about that till we get to that point.

:icon="item.icon"
:name="item.name"
:is-nav-bar-expanded="isNavBarExpanded"
Expand Down