-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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
chore: add v13 welcome page content #27549
chore: add v13 welcome page content #27549
Conversation
5727579
to
a59b58f
Compare
15f97f7
to
4fd745f
Compare
chore: add video link for v13 inside graphql context and root resolver. add migration tests outside of migration manager and update component tests to test aspects chore: update copy to most recent. Needs onlink deployments chore: opt for named slot in major version welcome as we still want to show the changes whether or not the video is present chore: fix video margin by shrinking the margin for a more appealing loadout chore: update onlinks for what is expected in deploy chore: adjust margins around video, move video up, and reduce margin on major welcome title chore: bump release date to 8/29 chore: update stubbed query to reference actual video
56a4ca7
to
fca2b37
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Content looks good, I didn't do a thorough code review and would like someone to do that and get it up and running in their review.
Sounds good. The
inside the |
I'll plan to pull this down later today and review! |
@tgriesser I want to double check and make sure I handled the resolution properly in the graphQL resolver. |
/> | ||
> | ||
<template | ||
v-if="videoHtml" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will this pass if videoHtml is ''?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It does not. v-if
only evaluates with the expression is truthy https://vuejs.org/guide/essentials/conditional.html#v-if
@@ -0,0 +1,10 @@ | |||
const { defineConfig } = require('cypress') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where do these get run?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in the e2e migration test
describe('v13 migration welcome page with video', () => { |
<div class="mb-[16px]"> | ||
<ExternalLink | ||
href="https://on.cypress.io/changelog#12-0-0" | ||
href="https://on.cypress.io/changelog?utm_source=splash-page&utm_campaign=v13#13-0-0" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Generally we use only 2 values for utm_source
- Binary: App
or Binary: Launchpad
. Is this intentionally different from that? I realize it's not documented well. There's a sheet that wrangles these values, it would be good to add these there. Medium seems more suitable than "source" for "splash page" based on the existing pattern.
But also curious - is there a particular person expecting to do some analysis later based on these params, or are we more playing it safe?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm probably not the best person to ask about the utm_source
, but it was in the approved copy for the splash page. I wonder if it makes sense to update the utm_source
to Binary: App
and leverage medium
for the splash page
. Heres the copy for reference. I can ask the GTM team in slack
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good idea to clarify in Slack. Being consistent is nice but there's not a hard and fast rule.
Co-authored-by: Mark Noonan <mark@cypress.io>
Co-authored-by: Mark Noonan <mark@cypress.io>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This all looks good to me, approving with the assumption that we figure out the UTM params before merge.
29 flaky tests on run #50422 ↗︎
Details:
e2e/origin/config_env.cy.ts • 1 flaky test • 5x-driver-firefox
e2e/origin/commands/assertions.cy.ts • 1 flaky test • 5x-driver-firefox
cypress/cypress.cy.js • 3 flaky tests • 5x-driver-firefox
scaffold-component-testing.cy.ts • 1 flaky test • launchpad-e2e
config-files-error-handling.cy.ts • 1 flaky test • launchpad-e2e
The first 5 flaky specs are shown, see all 17 specs in Cypress Cloud. This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. |
…abortcontroller is difficult to test with current ctx setup
d31067f
to
2317e62
Compare
Released in This comment thread has been locked. If you are still experiencing this issue after upgrading to |
Additional details
Adds the v13 welcome/splash page with updated content
Steps to test
How has the user experience changed?
PR Tasks
cypress-documentation
?type definitions
?