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: replace reconfigure button on settings page with link to config doc #21077

Merged
merged 12 commits into from
Apr 14, 2022
13 changes: 0 additions & 13 deletions packages/app/cypress/e2e/settings.cy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,19 +27,6 @@ describe('App: Settings', () => {
cy.get('button').contains('Log In')
})

it('can reconfigure a project', () => {
cy.startAppServer('e2e')
cy.visitApp('settings')
cy.withCtx((ctx, o) => {
o.sinon.stub(ctx.actions.project, 'reconfigureProject')
})

cy.findByText('Reconfigure Project').click()
cy.withRetryableCtx((ctx) => {
expect(ctx.actions.project.reconfigureProject).to.have.been.called
})
})

describe('Cloud Settings', () => {
it('shows the projectId section when there is a projectId', () => {
cy.withCtx(async (ctx, o) => {
Expand Down
7 changes: 7 additions & 0 deletions packages/app/src/settings/SettingsContainer.cy.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -50,4 +50,11 @@ describe('<SettingsContainer />', { viewportHeight: 800, viewportWidth: 900 }, (

cy.findByText(defaultMessages.settingsPage.projectId.title).should('not.exist')
})

it('renders footer with CTA button', () => {
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 used a component test instead of an e2e test to get some experience with CT 😄 in general, when both approaches are equal, do we have a preference for one or the other?

Copy link
Contributor

Choose a reason for hiding this comment

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

For verifying text content I prefer to keep it in the component tests.

mountSettingsContainer()
cy.contains('p', defaultMessages.settingsPage.footer.text)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
cy.contains('p', defaultMessages.settingsPage.footer.text)
cy.contains('p', defaultMessages.settingsPage.footer.text.replace('{testingType}', 'E2E'))

cy.contains('a', defaultMessages.settingsPage.footer.button)
.should('have.attr', 'href', defaultMessages.settingsPage.footer.buttonLink)
})
})
16 changes: 2 additions & 14 deletions packages/app/src/settings/SettingsContainer.vue
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@
variant="outline"
Copy link
Contributor

Choose a reason for hiding this comment

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

Above for the t('settingsPage.footer.text') should be modified to include the testing type since it's currently wired to only show component. Something like:

const footerText = computed(() => t('settingsPage.footer.text', { testingType: props.gql.currentProject?.currentTestingType === 'component' ? 'component' : 'E2E' }))

Then you can render {{ footerText }}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Great callout!

:prefix-icon="SettingsIcon"
prefix-icon-class="icon-dark-gray-500 icon-light-gray-50 group-hocus:icon-dark-indigo-400 group-hocus:icon-light-indigo-50"
@click="reconfigure"
:href="t('settingsPage.footer.buttonLink')"
>
{{ t('settingsPage.footer.button') }}
</Button>
Expand All @@ -52,7 +52,7 @@

<script lang="ts" setup>
import { useI18n } from '@cy/i18n'
import { gql, useMutation } from '@urql/vue'
import { gql } from '@urql/vue'
import Button from '@cy/components/Button.vue'
import ExternalEditorSettings from './device/ExternalEditorSettings.vue'
import ProxySettings from './device/ProxySettings.vue'
Expand All @@ -61,20 +61,13 @@ import ProjectSettings from './project/ProjectSettings.vue'
import CloudSettings from '../settings/project/CloudSettings.vue'
import TestingPreferences from './device/TestingPreferences.vue'
import type { SettingsContainerFragment } from '../generated/graphql'
import { SettingsContainer_ReconfigureProjectDocument } from '../generated/graphql'
import IconLaptop from '~icons/cy/laptop_x24.svg'
import IconOdometer from '~icons/cy/object-odometer_x24.svg'
import IconFolder from '~icons/cy/folder-outline_x24.svg'
import SettingsIcon from '~icons/cy/settings_x16.svg'

const { t } = useI18n()

gql`
mutation SettingsContainer_ReconfigureProject {
reconfigureProject
}
`

gql`
fragment SettingsContainer on Query {
...TestingPreferences
Expand All @@ -91,9 +84,4 @@ const props = defineProps<{
gql: SettingsContainerFragment
}>()

const openElectron = useMutation(SettingsContainer_ReconfigureProjectDocument)

function reconfigure () {
openElectron.executeMutation({})
}
</script>
5 changes: 3 additions & 2 deletions packages/frontend-shared/src/locales/en-US.json
Original file line number Diff line number Diff line change
Expand Up @@ -410,8 +410,9 @@
}
},
"footer": {
"text": "You can reconfigure the settings for this project if you're experiencing issues with your Cypress configuration.",
"button": "Reconfigure Project"
"text": "You can reconfigure the component testing settings for this project if you're experiencing issues with your Cypress configuration.",
rachelruderman marked this conversation as resolved.
Show resolved Hide resolved
"button": "Configuration Guide",
"buttonLink": "https://docs.cypress.io/guides/references/configuration"
}
},
"runs": {
Expand Down