-
Notifications
You must be signed in to change notification settings - Fork 245
feat: setting up early indexes experimentation and experiment viewed event CLOUDP-311776 #6860
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
Merged
Merged
Changes from 7 commits
Commits
Show all changes
16 commits
Select commit
Hold shift + click to select a range
61384ae
setting up experimentation file + receiving data from mms
rubydong 4a08032
update modal title, size, and description
rubydong fda1d08
simplifiedexperiment viewed func
rubydong 1c42217
Merge branch 'main' into cloudp-311776
rubydong 2b288bb
moved growth-experiments to telemetry folder + updated hook name
rubydong 22fd8f6
Merge remote-tracking branch 'origin/main' into cloudp-311776
rubydong b10c782
Merge branch 'cloudp-311776' of https://github.com/mongodb-js/compass…
rubydong a3990c3
Merge remote-tracking branch 'origin/main' into cloudp-311776
rubydong da566de
moved func to provider
rubydong 18168b9
created a CreateIndexModalHeader component
rubydong 07c902d
added unit tests for create-index-modal-header
rubydong 31b419d
fire EV if modal is open + removed preferences from AtlasOrgPreferences
rubydong 5fa1a06
remove extra space in modal-header
rubydong 8001399
resolve merge conflicts
rubydong 349d10e
remove extra h3 and body export
rubydong 0307155
updated styling
rubydong File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,29 @@ | ||
| import { | ||
| useTrackOnChange, | ||
| type TrackFunction, | ||
| } from '@mongodb-js/compass-telemetry/provider'; | ||
Anemy marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
|
|
||
| export enum TestName { | ||
| earlyJourneyIndexesGuidance = 'EARLY_JOURNEY_INDEXES_GUIDANCE_20250328', | ||
| } | ||
|
|
||
| export const useFireExperimentViewed = ({ | ||
| testName, | ||
| shouldFire = true, | ||
| }: { | ||
| testName: TestName; | ||
| shouldFire?: boolean; | ||
| }) => { | ||
| useTrackOnChange( | ||
| (track: TrackFunction) => { | ||
| if (!shouldFire) { | ||
| return; | ||
| } | ||
| track('Experiment Viewed', { | ||
| test_name: testName, | ||
| }); | ||
| }, | ||
| [], | ||
| undefined | ||
| ); | ||
| }; | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.
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.
The purpose of compass-components package is to have uniform, consistent components for all places where they are used in compass and this goes very much against it. For the experiment if we really need to do this, this should really just stay scoped to the create index modal, but generally speaking this is just not aligned with how we are building compass: why only this single modal needs a custom header style that will not be aligned with everything else in the application?
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 has been approved by Claudia and Diancheng also reviewed the designs! also i tried to make it more generic by saying 'useLeafyGreenStyling', which is consistent with leafy green at least. if useLeafyGreenStyling is false (which it is by default) then it'd be uniform for all the other modals!
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.
More generic configurable option is not the same as uniform styles (more variants less uniformity) 🙂 If Claudia signed off on switching all modals to this style because it's aligned with leafygreen more, let's just do it. Adding a generic option that we will be using only once in one specific place for an experiment that can be scrapped is not how we approach adding new components or properties to this particular package
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.
seems like Claudia is OOO and i'm not sure if there's a plan to switch the others just that she signed off on the design for this specific modal. would it be better if i make a separate component for modal header or update the prop name to be specific to index guidance?
Uh oh!
There was an error while loading. Please reload this page.
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.
Better would be to create a separate component in packages/compass-indexes/src/components/create-index-modal/create-index-modal.tsx file and use it instead of ModalHeader and not modify compass-components, it's just two existing leafygreen components you need to stack together, there is absolutely no point to generalize this if we're not planning to update other parts of Compass to use it. Just to stress again: we don't put one-off components in the compass-components package, we don't put feature specific props in the components in this package