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

Conversation

rachelruderman
Copy link
Contributor

@rachelruderman rachelruderman commented Apr 13, 2022

User facing changelog

n/a

Additional details

Since we have removed reconfiguring the testing type from the 10.0 release, the Reconfigure Project button in the Settings page should link to the Configuration Guide in the Cypress docs instead.

How has the user experience changed?

Before:
Screen Shot 2022-04-14 at 9 08 13 AM

After:
Screen Shot 2022-04-14 at 9 09 09 AM

PR Tasks

  • Have tests been added/updated?
  • [n/a] Has the original issue (or this PR, if no issue exists) been tagged with a release in ZenHub? (user-facing changes only)
  • [n/a] Has a PR for user-facing changes been opened in cypress-documentation?
  • [n/a] Have API changes been updated in the type definitions?
  • [n/a] Have new configuration options been added to the cypress.schema.json?

@cypress-bot
Copy link
Contributor

cypress-bot bot commented Apr 13, 2022

Thanks for taking the time to open a PR!

@rachelruderman
Copy link
Contributor Author

Note: I haven't marked this for a release since it is not technically user-facing. This UI change is not considered user-facing since the UI it changes has not been released yet. Plz let me know if you think differently 😃

@cypress
Copy link

cypress bot commented Apr 13, 2022



Test summary

17872 0 217 0Flakiness 1


Run details

Project cypress
Status Passed
Commit ed1ee3b
Started Apr 14, 2022 5:21 PM
Ended Apr 14, 2022 5:35 PM
Duration 13:54 💡
OS Linux Debian - 10.10
Browser Multiple

View run in Cypress Dashboard ➡️


Flakiness

cypress/e2e/commands/net_stubbing.cy.ts Flakiness
1 network stubbing > intercepting response > can throttle a proxy response using res.setThrottle

This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard

Copy link
Contributor

@davidmunechika davidmunechika left a comment

Choose a reason for hiding this comment

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

@@ -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.

Copy link
Contributor

@ZachJW34 ZachJW34 left a comment

Choose a reason for hiding this comment

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

There are two PRs removing the ability to reconfigure a project, this one and #21063 @davidmunechika. There is some backend code that should be cleaned up as well since it's now dead code, I think it should be removed in one of these PRs (whichever gets merged last) or a ticket should be created and a fast follow.

packages/frontend-shared/src/locales/en-US.json Outdated Show resolved Hide resolved
@@ -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!


it('renders footer with CTA button', () => {
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'))

@rachelruderman rachelruderman requested a review from ZachJW34 April 14, 2022 17:05
Copy link
Contributor

@ZachJW34 ZachJW34 left a comment

Choose a reason for hiding this comment

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

Nice work!

@rachelruderman rachelruderman merged commit b21ecfb into 10.0-release Apr 14, 2022
@rachelruderman rachelruderman deleted the unify-1532 branch April 14, 2022 18:11
@rachelruderman rachelruderman restored the unify-1532 branch April 14, 2022 20:39
tgriesser added a commit that referenced this pull request Apr 20, 2022
…e-config

* 10.0-release:
  fix: make error on integration folder point to e2e (#20853)
  fix(unify): Update Cypress Dashboard Service Link in Login Modal (#21084)
  chore: fix windows node_modules install step (#21098)
  fix: webpack integration tests for app w webpack-dev-server-fresh (#21093)
  fix: move non spec files on migration (#21054)
  Bumping electron version in root
  chore(deps): Bumping electron dependency to 15.5.1 (#21072)
  fix: resolves correctly specPattern (#21057)
  feat: replace reconfigure button on settings page with link to config doc (#21077)
  feat(launchpad): update CT setup and config scaffolding (#20893)
  fix: cy.type('{enter}') on <input> elements submits the form correctly after Firefox 98. (#21042)
  chore: making the npm deps for vue, react, and vue2 use 0.0.0-dev (#21081)
  fix(cli): show additional mitigation steps for max path length error (#21047)
  fix: Plugin error when sending on disconnected IPC channel (#21011)
  chore: add internal types for cy.session command (#21028)
  chore: Update Chrome (stable) to 100.0.4896.88 (#21043)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants