-
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
Dashboard Integration Smart Record Run Notification #22935
Comments
@pstakoun Other "smart notification" issues don't reference the "only show if it's been 2 days" behavior - is that intended to be applied to those as well? Going to need a bit more info about how this applies - is the 2-day restriction based on showing a notification that "comes before" this one in the setup workflow, or does it also apply to itself (only show this prompt if it's been 2 days since we last showed it)? And I assume the dismiss behavior overrides the 2 day restriction so we never show it again regardless of time period? |
@mike-plummer I would say no, only this one should have the "2 days" behavior, but I've been debating that a bit. You are correct in assuming the dismiss behavior overrides the 2 day restriction. Anything else unclear here? |
Hey team! Please add your planning poker estimate with ZenHub @astone123 @lmiller1990 @marktnoonan @mike-plummer @rockindahizzy @warrensplayer @ZachJW34 |
Looks like a few of the Dashboard Integration features have a "10 tests" requirement. If you pick this up, see if anyone else has implemented the "10 tests" check first, to avoid duplicate work 👍. |
Is the A/B requirement dropped? The linked doc does not show the variants as far as I could tell. It looks like we are about to starting saving a bunch more project-level state, which #22755 will help with by providing a "save project" state helper instead of a field-specific helper for each thing we need to save. This ticket would leverage that, for persisting dismissal, tracking last shown times, etc. Which might reduce the work's complexity. But that ticket is in this same sprint, and I'd be cautious about interdependencies between separate tickets in the same sprint. We should keep that to a minimum, and all things being equal I think this is a good candidate to move to the next sprint. |
@marktnoonan each prompt has A and B variants (one has a C as well) - this one has it for the CTA |
This is high priority, so would push back against moving it out of the sprint and would rather move out others. |
Ah cool @pstakoun I was reading the document incorrectly, ty. Let's review priority/order as a whole when everything is estimated. |
I have removed the "2 days" limitation to simplify logic |
@pstakoun Disregard, I forgot we have a warning banner for that states that would preclude this banner being displayed |
Completed in #23256 |
Released in This comment thread has been locked. If you are still experiencing this issue after upgrading to |
This is an info banner as per the designs in Figma.
The banner should show the record command.
If the banner is dismissed, it should never be shown again.
The conditions for this banner to be visible are:
shouldShowPrompt
inHeaderBarContent.vue
The copy for this banner should be the "A" variant described in this doc.
Please reference this sheet for info about all relevant states.
The text was updated successfully, but these errors were encountered: