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

feat: auto-collapse sidebar for window width < lg #22393

Merged
merged 4 commits into from
Jul 16, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 16 additions & 14 deletions packages/app/cypress/e2e/settings.cy.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import type { SinonStub } from 'sinon'

const SidebarSettingsLinkSelector = '[data-cy="sidebar-link-settings-page"]'

describe('App: Settings', () => {
before(() => {
cy.scaffoldProject('todos', { timeout: 50 * 1000 })
Expand All @@ -12,7 +14,7 @@ describe('App: Settings', () => {
it('visits settings page', () => {
cy.startAppServer('e2e')
cy.visitApp()
cy.findByText('Settings').click()
cy.get(SidebarSettingsLinkSelector).click()

cy.get('div[data-cy="app-header-bar"]').should('contain', 'Settings')
cy.findByText('Device Settings').should('be.visible')
Expand All @@ -22,7 +24,7 @@ describe('App: Settings', () => {
it('shows a button to log in if user is not connected', () => {
cy.startAppServer('e2e')
cy.visitApp()
cy.findByText('Settings').click()
cy.get(SidebarSettingsLinkSelector).click()
cy.findByText('Project Settings').click()
cy.get('button').contains('Log In')
})
Expand All @@ -35,7 +37,7 @@ describe('App: Settings', () => {

cy.startAppServer('e2e')
cy.visitApp()
cy.findByText('Settings').click()
cy.get(SidebarSettingsLinkSelector).click()
cy.findByText('Dashboard Settings').click()
cy.findByText('Project ID').should('be.visible')
cy.get('[data-cy="code-box"]').should('contain', 'fromCli')
Expand All @@ -51,7 +53,7 @@ describe('App: Settings', () => {
cy.loginUser()

cy.visitApp()
cy.findByText('Settings').click()
cy.get(SidebarSettingsLinkSelector).click()
cy.findByText('Dashboard Settings').click()
cy.findByText('Record Key').should('be.visible')
})
Expand All @@ -61,7 +63,7 @@ describe('App: Settings', () => {
cy.loginUser()

cy.visitApp()
cy.findByText('Settings').click()
cy.get(SidebarSettingsLinkSelector).click()
cy.findByText('Dashboard Settings').click()
cy.get('[data-cy="code-box"]').should('contain', '***')
cy.get('[aria-label="Record Key Visibility Toggle"]').click()
Expand Down Expand Up @@ -99,7 +101,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.findByTestId('sidebar-link-settings-page').click()
cy.get(SidebarSettingsLinkSelector).click()
cy.contains('Dashboard Settings').click()
// Assert the data is not there before it arrives
cy.contains('Record Key').should('not.exist')
Expand Down Expand Up @@ -139,7 +141,7 @@ describe('App: Settings', () => {
cy.loginUser()

cy.visitApp()
cy.findByText('Settings').click()
cy.findByTestId('sidebar-link-settings-page').click()
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm, should this be cy.get(SidebarSettingsLinkSelector)? Or it's probably more typical for all the others to be findByTestId('sidebar-link-settings-page'), since the selector is a test id.

cy.findByText('Project Settings').click()
cy.get('[data-cy="file-match-indicator"]').contains('2 Matches')
cy.get('[data-cy="spec-pattern"]').contains('**/*.cy.{js,jsx,ts,tsx}')
Expand All @@ -157,7 +159,7 @@ describe('App: Settings', () => {
cy.loginUser()

cy.visitApp()
cy.findByText('Settings').click()
cy.get(SidebarSettingsLinkSelector).click()
cy.findByText('Project Settings').click()
cy.get('[data-cy="file-match-indicator"]').contains('41 Matches')
cy.get('[data-cy="spec-pattern"]').contains('tests/**/*')
Expand All @@ -168,7 +170,7 @@ describe('App: Settings', () => {
cy.loginUser()

cy.visitApp()
cy.findByText('Settings').click()
cy.get(SidebarSettingsLinkSelector).click()
cy.findByText('Project Settings').click()
cy.get('[data-cy="settings-experiments"]').within(() => {
cy.validateExternalLink({
Expand Down Expand Up @@ -231,7 +233,7 @@ describe('App: Settings', () => {
cy.loginUser()

cy.visitApp()
cy.findByText('Settings').click()
cy.get(SidebarSettingsLinkSelector).click()
cy.findByText('Project Settings').click()
cy.get('[data-cy="config-code"]').contains('{')
})
Expand All @@ -241,7 +243,7 @@ describe('App: Settings', () => {
cy.loginUser()

cy.visitApp()
cy.findByText('Settings').click()
cy.get(SidebarSettingsLinkSelector).click()
cy.findByText('Project Settings').click()
cy.get('[data-cy="config-legend"]').within(() => {
cy.get('.bg-gray-50').contains('default')
Expand Down Expand Up @@ -284,7 +286,7 @@ describe('App: Settings', () => {
cy.loginUser()

cy.visitApp()
cy.findByText('Settings').click()
cy.get(SidebarSettingsLinkSelector).click()
cy.findByText('Project Settings').click()
cy.get('[data-cy="config-legend"]').within(() => {
cy.get('.bg-gray-50').contains('default')
Expand Down Expand Up @@ -410,7 +412,7 @@ describe('App: Settings without cloud', () => {
cy.startAppServer('component')

cy.visitApp()
cy.findByText('Settings').click()
cy.get(SidebarSettingsLinkSelector).click()
cy.findByText('Dashboard Settings').click()
cy.findByText('Project ID').should('exist')
cy.contains('button', 'Log in to the Cypress Dashboard').should('be.visible')
Expand All @@ -422,7 +424,7 @@ describe('App: Settings without cloud', () => {
cy.startAppServer('component')

cy.visitApp()
cy.findByText('Settings').click()
cy.get(SidebarSettingsLinkSelector).click()
cy.findByText('Project Settings').click()

cy.get('[data-cy=config-code]').within(() => {
Expand Down
2 changes: 1 addition & 1 deletion packages/app/cypress/e2e/sidebar_navigation.cy.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import type { SinonStub } from 'sinon'

describe('Sidebar Navigation', () => {
describe('Sidebar Navigation', { viewportWidth: 1280 }, () => {
context('accessibility', () => {
beforeEach(() => {
cy.scaffoldProject('todos')
Expand Down
6 changes: 3 additions & 3 deletions packages/app/cypress/e2e/specs_list_e2e.cy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,11 @@ describe('App: Spec List (E2E)', () => {

it('shows the "Specs" navigation as highlighted in the lefthand nav bar', () => {
cy.findByTestId('sidebar').within(() => {
cy.findByText('Specs').should('be.visible')
cy.findByText('Specs').click()
cy.findByTestId('sidebar-link-specs-page').should('be.visible')
cy.findByTestId('sidebar-link-specs-page').click()
})

cy.get('[data-selected="true"]').contains('Specs').should('be.visible')
cy.findByTestId('sidebar-link-specs-page').find('[data-selected="true"]').should('be.visible')
})

it('displays the App Top Nav', () => {
Expand Down
28 changes: 27 additions & 1 deletion packages/app/src/navigation/SidebarNavigation.cy.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ function mountComponent (initialNavExpandedVal = true) {
}

describe('SidebarNavigation', () => {
it('expands the bar when clicking the expand button', () => {
it('expands the bar when clicking the expand button', { viewportWidth: 1280 }, () => {
mountComponent()

cy.findByText('test-project').should('not.be.visible')
Expand All @@ -31,6 +31,32 @@ describe('SidebarNavigation', () => {
cy.percySnapshot()
})

it('automatically collapses when viewport decreases < 1024px', () => {
cy.viewport(1280, 1000)
mountComponent()

// Collapsed by default
cy.findByText('test-project').should('not.be.visible')

// Expand
cy.findByLabelText(defaultMessages.sidebar.toggleLabel.collapsed, {
selector: 'button',
}).click()

// Verify expanded - project title should display
cy.findByText('test-project').should('be.visible')

// Shrink viewport, should collapse
cy.viewport(1000, 1000)

// Project title should not be visible anymore
cy.findByText('test-project').should('not.be.visible')
// Expand control should not be available
cy.findByLabelText(defaultMessages.sidebar.toggleLabel.collapsed, {
selector: 'button',
}).should('not.exist')
})

it('shows tooltips on hover', () => {
mountComponent(false)
cy.findByTestId('sidebar-header').trigger('mouseenter')
Expand Down
11 changes: 8 additions & 3 deletions packages/app/src/navigation/SidebarNavigation.vue
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,7 @@ import CypressLogo from '@packages/frontend-shared/src/assets/logos/cypress_s.pn
import { useI18n } from '@cy/i18n'
import { useRoute } from 'vue-router'
import SidebarNavigationHeader from './SidebarNavigationHeader.vue'
import { useWindowSize } from '@vueuse/core'

const { t } = useI18n()

Expand Down Expand Up @@ -140,6 +141,8 @@ query SideBarNavigation {
}
`

const NAV_EXPAND_MIN_SCREEN_WIDTH = 1024

const query = useQuery({ query: SideBarNavigationDocument })

const setPreferences = useMutation(SideBarNavigation_SetPreferencesDocument)
Expand All @@ -148,20 +151,22 @@ const bindingsOpen = ref(false)

const route = useRoute()

const navIsAlwaysCollapsed = computed(() => route.meta?.navBarExpandedAllowed !== false)
const navIsAlwaysCollapsed = computed(() => width.value >= NAV_EXPAND_MIN_SCREEN_WIDTH && route.meta?.navBarExpandedAllowed !== false)

const isNavBarExpanded = ref(true)

const { width } = useWindowSize()

watchEffect(() => {
if (route.meta?.navBarExpandedAllowed === false) {
if (width.value < NAV_EXPAND_MIN_SCREEN_WIDTH || route.meta?.navBarExpandedAllowed === false) {
isNavBarExpanded.value = false
} else {
isNavBarExpanded.value = query.data?.value?.localSettings.preferences.isSideNavigationOpen ?? true
}
})

const toggleNavbarIfAllowed = () => {
if (route.meta?.navBarExpandedAllowed === false) {
if (width.value < NAV_EXPAND_MIN_SCREEN_WIDTH || route.meta?.navBarExpandedAllowed === false) {
return
}

Expand Down