misc: ACI empty state slideshows #26692
Conversation
mike-plummer
force-pushed
the
mikep/26181-aci-empty-states
branch
from
d5fabc2 to
b5597e1
Compare
mike-plummer
changed the title
8 failed and 8 flaky tests on run #46064 ↗︎
Details:
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Test | Artifacts | |
|---|---|---|
commands/command_option_immutability.cy.js • 0 failed tests • 5x-driver-firefox
| Test | Artifacts | |
|---|---|---|
The first 5 failed specs are shown, see all 887 specs in Cypress Cloud.
commands/net_stubbing.cy.ts • 1 flaky test • 5x-driver-firefox
| Test | Artifacts | |
|---|---|---|
| network stubbing > intercepting request > can delay and throttle a StaticResponse |
Output
|
|
create-from-component.cy.ts • 1 flaky test • app-e2e
| Test | Artifacts | |
|---|---|---|
| ... > runs generated spec |
Output
Screenshots
Video
|
|
commands/net_stubbing.cy.ts • 1 flaky test • 5x-driver-electron
| Test | Artifacts | |
|---|---|---|
| network stubbing > intercepting request > can delay and throttle a StaticResponse |
Output
Video
|
|
commands/net_stubbing.cy.ts • 2 flaky tests • 5x-driver-chrome:beta
| Test | Artifacts | |
|---|---|---|
| network stubbing > intercepting request > can delay and throttle a StaticResponse |
Output
Video
|
|
| ... > stops waiting when an fetch request is canceled |
Output
Video
|
|
commands/net_stubbing.cy.ts • 1 flaky test • 5x-driver-chrome
| Test | Artifacts | |
|---|---|---|
| network stubbing > intercepting request > can delay and throttle a StaticResponse |
Output
Video
|
|
The first 5 flaky specs are shown, see all 6 specs in Cypress Cloud.
This comment has been generated by cypress-bot as a result of this project's GitHub integration settings.
| import RecordPromptAdapter from '@cy/gql-components/RecordPromptAdapter.vue' | ||
| import { DEBUG_SLIDESHOW } from '../utils/constants' | ||
| import GuideCard1 from '../guide/GuideCard1.vue' |
There was a problem hiding this comment.
Just a suggestion for future PRs, I think we can do this like you often see in React using a namespace:
export const Guide = {
Card1,
Card2
}
// ...
<template>
<Guide.Card1 />
<Guide.Card2 />
</template>There was a problem hiding this comment.
This is fine too, though 👍
There was a problem hiding this comment.
Love it, definitely will restructure. I knew exactly how I would structure all of this state & these components in React but was struggling to map that into the "Vue way"
There was a problem hiding this comment.
Sure, no problem - not sure why this is less common in Vue world, same patterns should generally work.
There was a problem hiding this comment.
LMK when you do this (or if you decide not to) and I can approve, everything else is looking pretty good to me.
There was a problem hiding this comment.
I ended up writing a simple wrapper for each of the sets of cards (DebugGuide, DebugTour, etc) so the card components don't have to be referenced en-masse outside that directory. Given how the different slideshows were structured I didn't end up seeing much organizational benefit from the namespacing suggestion
| const { t } = useI18n() | ||
|
|
||
| defineProps<{ | ||
| action: () => void |
There was a problem hiding this comment.
We've got this defineProps a lot. It's quite simple, but might be worth trying to find an abstraction (I suggested one, but it might not work here - I didn't try it yet).
Wouldn't consider this to be a blocker, but food for thought - do you think these duplication is worth the effort to avoid?
packages/frontend-shared/src/gql-components/promo/PromoAction.vue
Outdated
Show resolved
Hide resolved
packages/frontend-shared/src/gql-components/promo/PromoCard.vue
Outdated
Show resolved
Hide resolved
* Implement promo on `DebugNoProject` view * Move Slideshow & Promo to `packages/app` * Consolidate Guide & Tour setup into wrapper components * Fix styling * Delete unused assets
* Use `grid` layout for `PromoCard` * Fix button styling in `PromoAction` * Update changelog
There was a problem hiding this comment.
Works great, left a few comments, ping me for a re-review when you are happy with the stater of things @mike-plummer 💯
Co-authored-by: Stokes Player <[email protected]>
|
@mike-plummer Played around with styles and came up with this PR into this branch: #26733 Let me know what you think. I still want to go through the code one more time and verify the telemetry and links, but getting close. Also, we should grab Chris France to do quick acceptance test. |
|
I'm going to out, so I won't be able to respond to any comments in a timely fashion - I played with the PR a bit more and it's looking good. I'll leave a tentative approval just to avoid needing a third person to go through the entire thing. I hadn't caught the weirdness with the responsive design in my first pass, but glad we got it fixed. |
There was a problem hiding this comment.
Looks good to me. Still a strange issue with scrolling when the viewport is too short where it will not scroll into view the bottom padding. I think it has something to do with the nested divs that all have to have height of 100% in order to center the component. Not worth fighting any. more.
|
Released in This comment thread has been locked. If you are still experiencing this issue after upgrading to |
Additional details
Replacing four views with a new slideshow design from Figma (see referenced issue for links)
For slides I just did the MVP of cross-slide fade for now, described "extra credit" enhancements can be implemented later if determined to be worth it.
Each
Promowill record an event when it is seen, CTAs & links include UTM params to determine source.Steps to test
Easiest way to test this is to use
DebugEmptyStates.cy.tsxandRunsContainer.cy.tsxspecsHow has the user experience changed?
Debug: Not Logged In
debug_notLoggedIn.mov
Debug: No Recorded Runs
Note: Mocked state for this video so no record key is showing in command
debug_empty.mov
Runs: Not Connected
runs_connect.mov
Runs: No Recorded Runs
Note: Mocked state for this video so no record key is showing in command
runs_empty.mov
PR Tasks
cypress-documentation?type definitions?