feat: setting up early indexes experimentation and experiment viewed event CLOUDP-311776 #6860

rubydong · 2025-04-10T15:57:19Z

Description

Before:

After (made modal bigger, updated the header title, removed db + collection name, added a description):

Experiment Viewed event added:

Checklist

New tests and/or benchmarks are included
Documentation is changed or added
If this change updates the UI, screenshots/videos are added and a design review is requested
I have signed the MongoDB Contributor License Agreement (https://www.mongodb.com/legal/contributor-agreement)

Motivation and Context

Bugfix
New feature
Dependency update
Misc

Types of changes

Backport Needed
Patch (non-breaking change which fixes an issue)
Minor (non-breaking change which adds functionality)
Major (fix or feature that would cause existing functionality to change)

gribnoysup · 2025-04-11T08:16:25Z

packages/compass-components/src/components/modals/modal-header.tsx

+      {useLeafyGreenStyling ? (
+        <H3 data-testid="modal-title" id="modal-title">
+          {title}
+        </H3>
+      ) : (
+        <h1
+          className={cx(titleStyle, darkMode && titleStyleDark)}
+          data-testid="modal-title"
+          id="modal-title"
+        >
+          {title}
+        </h1>
+      )}


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?

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!

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

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?

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

packages/compass-utils/src/growth-experiments.ts

…into cloudp-311776

packages/compass-telemetry/src/index.ts

packages/compass-telemetry/src/growth-experiments.ts

packages/compass-indexes/src/components/create-index-modal/create-index-modal.tsx

packages/compass-preferences-model/src/preferences-schema.ts

Anemy

Looking good, left two small comments/questions.

Anemy · 2025-04-18T14:49:25Z

packages/compass-components/src/index.ts

@@ -83,6 +83,7 @@ export { VisuallyHidden } from '@react-aria/visually-hidden';
 export { openToast, closeToast, ToastArea } from './hooks/use-toast';

 export { breakpoints, spacing } from '@leafygreen-ui/tokens';
+export { H3, Body } from '@leafygreen-ui/typography';


We're already exporting these from the leafygreen.tsx file:

compass/packages/compass-components/src/components/leafygreen.tsx

Line 162 in 7371e1b

H3,

So we can remove this.

Suggested change

export { H3, Body } from '@leafygreen-ui/typography';

Anemy · 2025-04-18T14:52:07Z

packages/compass-indexes/src/components/create-index-modal/create-index-modal.tsx

+
+  useFireExperimentViewed({
+    testName: TestName.earlyJourneyIndexesGuidance,
+    shouldFire: enableInIndexesGuidanceExp && isVisible,


Maybe I'm understanding this wrong, should this be showIndexesGuidanceVariant and not enableInIndexesGuidanceExp?

Suggested change

shouldFire: enableInIndexesGuidanceExp && isVisible,

shouldFire: showIndexesGuidanceVariant && isVisible,

nope this should fire for when user is in the control as well so it should be enableInIndexesGuidanceExp

Should we track that they're in the control then? Or is there already a way we differentiate that they are shown/not shown the new experience?

this tracks if they are in the control as well, if they are in the experiment enableInIndexesGuidanceExp is true. i need to confirm once i get the latest into our mms but i think by default we have code set up to send down whether user is in the variant or control

Would it make things easier if we track if they're in the control or shown the experience here as some metadata for the tracking event? If it's on the mms side I suppose we don't need to since we'll be able to correlate them anyway. Feel free to resolve!

Anemy · 2025-04-18T14:58:40Z

packages/compass-indexes/src/components/create-index-modal/create-index-modal-header.tsx

+
+      <Body
+        data-testid="create-index-modal-header-subtitle"
+        style={{ color: palette.gray.dark1 }}


Small nit, we typically avoid using style unless it's a dynamic value we're passing to it. Instead we would create a css style variable and pass that to the className like we do for headerStyle.

Anemy · 2025-04-18T15:00:42Z

packages/compass-indexes/src/components/create-index-modal/create-index-modal-header.tsx

+
+      <Body
+        data-testid="create-index-modal-header-subtitle"
+        style={{ color: palette.gray.dark1 }}


Should we account for dark mode here? Is there a reason we need to explicitly set the color here and not use the default color? Typically we try to default to the colors from the leafygreen typography components. They're also plugged into the dark mode provider so they automatically switch when it changes.
If we explicitly want to have a color here, and we want it to change based on the theme, you can look for the useDarkMode() hook in the code base as a reference.

designs for dark mode will come out but is not available right now, here's a ticket: https://jira.mongodb.org/browse/CLOUDP-313986.

i only followed the design for the dark gray color though and in the design it was that color

Anemy · 2025-04-18T15:01:33Z

packages/compass-indexes/src/components/create-index-modal/create-index-modal-header.tsx

+
+const CreateIndexModalHeader = () => {
+  return (
+    <div className={cx(headerStyle)}>


Suggested change

<div className={cx(headerStyle)}>

<div className={headerStyle}>

Anemy · 2025-04-18T15:02:47Z

packages/compass-indexes/src/components/create-index-modal/create-index-modal-header.tsx

+import React from 'react';
+
+const headerStyle = css({
+  padding: spacing[5],


Suggested change

padding: spacing[5],

padding: spacing[800],

nit The single digit values here are deprecated (we'll be updating all of them in #6871 )

rubydong added 3 commits April 8, 2025 16:15

setting up experimentation file + receiving data from mms

61384ae

update modal title, size, and description

4a08032

simplifiedexperiment viewed func

Loading
Loading status checks…

fda1d08

github-actions bot added the feat label Apr 10, 2025

rubydong added the no release notes label Apr 10, 2025

Merge branch 'main' into cloudp-311776

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.

GPG key ID: B5690EEEBB952194

Verified
Learn about vigilant mode

Loading
Loading status checks…

1c42217

gribnoysup reviewed Apr 11, 2025

View reviewed changes

packages/compass-utils/src/growth-experiments.ts Outdated Show resolved Hide resolved

gribnoysup reviewed Apr 11, 2025

View reviewed changes

packages/compass-utils/src/growth-experiments.ts Outdated Show resolved Hide resolved

rubydong added 3 commits April 11, 2025 14:03

moved growth-experiments to telemetry folder + updated hook name

2b288bb

Merge remote-tracking branch 'origin/main' into cloudp-311776

22fd8f6

Merge branch 'cloudp-311776' of https://github.com/mongodb-js/compass …

Loading
Loading status checks…

b10c782

…into cloudp-311776

gribnoysup reviewed Apr 14, 2025

View reviewed changes

packages/compass-telemetry/src/index.ts Outdated Show resolved Hide resolved

gribnoysup reviewed Apr 14, 2025

View reviewed changes

packages/compass-telemetry/src/growth-experiments.ts Outdated Show resolved Hide resolved

rubydong added 2 commits April 14, 2025 12:56

Merge remote-tracking branch 'origin/main' into cloudp-311776

Loading
Loading status checks…

a3990c3

moved func to provider

Loading
Loading status checks…

da566de

rubydong marked this pull request as ready for review April 14, 2025 20:29

rubydong added 2 commits April 15, 2025 12:02

created a CreateIndexModalHeader component

Loading
Loading status checks…

18168b9

added unit tests for create-index-modal-header

Loading
Loading status checks…

07c902d

rubydong added the no-title-validation label Apr 15, 2025

gribnoysup reviewed Apr 16, 2025

View reviewed changes

packages/compass-indexes/src/components/create-index-modal/create-index-modal.tsx Outdated Show resolved Hide resolved

gribnoysup reviewed Apr 16, 2025

View reviewed changes

packages/compass-preferences-model/src/preferences-schema.ts Outdated Show resolved Hide resolved

rubydong added 3 commits April 16, 2025 11:33

fire EV if modal is open + removed preferences from AtlasOrgPreferences

Loading
Loading status checks…

31b419d

remove extra space in modal-header

Loading
Loading status checks…

5fa1a06

resolve merge conflicts

Loading
Loading status checks…

8001399

Anemy reviewed Apr 18, 2025

View reviewed changes

remove extra h3 and body export

Loading
Loading status checks…

349d10e

Anemy reviewed Apr 18, 2025

View reviewed changes

updated styling

Loading
Loading status checks…

0307155

Anemy approved these changes Apr 18, 2025

View reviewed changes

rubydong merged commit 5abf09b into main Apr 18, 2025
49 of 51 checks passed

rubydong deleted the cloudp-311776 branch April 18, 2025 16:31

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: setting up early indexes experimentation and experiment viewed event CLOUDP-311776 #6860

feat: setting up early indexes experimentation and experiment viewed event CLOUDP-311776 #6860

rubydong commented Apr 10, 2025 •

edited

Loading

gribnoysup Apr 11, 2025 •

edited

Loading

rubydong Apr 11, 2025

gribnoysup Apr 14, 2025

rubydong Apr 14, 2025

gribnoysup Apr 15, 2025 •

edited

Loading

Anemy left a comment •

edited

Loading

Anemy Apr 18, 2025

Anemy Apr 18, 2025

rubydong Apr 18, 2025

Anemy Apr 18, 2025

rubydong Apr 18, 2025

Anemy Apr 18, 2025

Anemy Apr 18, 2025

Anemy Apr 18, 2025

rubydong Apr 18, 2025

Anemy Apr 18, 2025

Anemy Apr 18, 2025

	shouldFire: enableInIndexesGuidanceExp && isVisible,
	shouldFire: showIndexesGuidanceVariant && isVisible,

	<div className={cx(headerStyle)}>
	<div className={headerStyle}>

feat: setting up early indexes experimentation and experiment viewed event CLOUDP-311776 #6860

feat: setting up early indexes experimentation and experiment viewed event CLOUDP-311776 #6860

Conversation

rubydong commented Apr 10, 2025 • edited Loading

Description

Checklist

Motivation and Context

Types of changes

gribnoysup Apr 11, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gribnoysup Apr 15, 2025 • edited Loading

Choose a reason for hiding this comment

Anemy left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rubydong commented Apr 10, 2025 •

edited

Loading

gribnoysup Apr 11, 2025 •

edited

Loading

gribnoysup Apr 15, 2025 •

edited

Loading

Anemy left a comment •

edited

Loading