-
Notifications
You must be signed in to change notification settings - Fork 221
feat: Add analytics for early index journey guidance CLOUDP-311789 #6976
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
Conversation
@@ -52,12 +52,14 @@ interface AccordionProps extends React.HTMLProps<HTMLButtonElement> { | |||
hintText?: string; | |||
open?: boolean; | |||
setOpen?: (newValue: boolean) => void; | |||
onOpenCallback?: () => void; |
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.
Adding callback to a callback prop name is redundant
onOpenCallback?: () => void; | |
onOpen?: () => void; |
@@ -70,6 +72,9 @@ function Accordion({ | |||
setOpenRef.current?.(newValue); | |||
return newValue; | |||
}); | |||
if (typeof onOpenCallback === 'function') { | |||
onOpenCallback(); |
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 will also be called on close, so the prop name is confusing, let's change that
| UpdateExportedEvent | ||
| UpdateExportOpenedEvent | ||
| ViewUpdatedEvent | ||
| SecretStorageNotAvailableEvent | ||
| FirstContentfulPaintEvent | ||
| LargestContentfulPaintEvent | ||
| FirstInputDelayEvent | ||
| CumulativeLayoutShiftEvent |
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.
For the future it would be nice to avoid unnecessary changes to the files like this one, we don't have any sorting rules for our codebase and this shift makes it really hard to understand whether or not there any meaningful changes here
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 misunderstood and thought that it was meant to be sorted but that it got messed up later on. i reverted the sorting
Description
https://jira.mongodb.org/browse/CLOUDP-311789
Following this list but note that some have already been added in previous PRs:
https://docs.google.com/document/d/1wf5x58vRqfTr_5xmhgM1j01VlIkvjnvQRiiyIo57L6Q/edit?tab=t.0#bookmark=kix.5fxzlr43voeh
I also alphabetized what's in telemetry-events.ts
Checklist
Motivation and Context
Types of changes