-
Notifications
You must be signed in to change notification settings - Fork 229
chore(compass-generative-ai): update Gen AI opt-in modal COMPASS-9593 #7164
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
base: main
Are you sure you want to change the base?
Conversation
textAlign: 'left', | ||
// TODO: The LG MarketingModal does not provide a way to disable a button | ||
// so this is a temporary workaround to make the button look disabled. | ||
const disableOptInButtonStyles = css({ |
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 seemed like the best solution, I can open a LG ticket and link it here if this sounds good.
@@ -62,6 +63,7 @@ export type AtlasOptInAttemptEndAction = { | |||
|
|||
export type AtlasOptInStartAction = { | |||
type: AtlasOptInActions.Start; | |||
isCloudOptIn: boolean; |
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.
not sure if this is the best way to go about this... but some differentiation will be needed for DE vs Compass opt-in. This can also be called isDataExplorerOptIn
or something
@@ -75,12 +67,14 @@ const getButtonText = (isOptInInProgress: boolean) => { | |||
export const AIOptInModal: React.FunctionComponent<OptInModalProps> = ({ | |||
isOptInModalVisible, | |||
isOptInInProgress, | |||
isCloudOptIn, | |||
onOptInModalClose, | |||
onOptInClick, | |||
projectId, | |||
}) => { | |||
const isProjectAIEnabled = usePreference('enableGenAIFeaturesAtlasProject'); |
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.
enableGenAIFeaturesAtlasProject
will eventually converge with the new Compass opt-in preference in #7129
// so this is a temporary workaround to make the button look disabled. | ||
const disableOptInButtonStyles = css({ | ||
button: { | ||
opacity: 0.5, |
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.
Would we want to try to match the LG disabled styles, as in https://github.com/mongodb/leafygreen-ui/blob/f9f9ed34f21b561986c46c502cf13ef82a0e7c3c/packages/button/src/Button/Button.styles.ts#L360?
Using opacity
here seems a bit odd and probably makes the button look a bit off compared to other disabled buttons?
className={isCloudOptIn ? disableOptInButtonStyles : undefined} | ||
onButtonClick={onConfirmClick} | ||
title="Opt-in Gen AI-Powered features" | ||
// @ts-expect-error - buttonText expects a string but supports ReactNode as well. |
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.
Also maybe a LG ticket for this?
Not to be merged but can be reviewed - pending updated copy text from product.
Updates the modals to the new design and prepares it for the case where we have the same opt-in modal for both DE & Compass. Differentiates this with a new action prop.
Changes here are minimal so we can merge this before the convergence of the opt-ins with #7129 happens. We will hide the mentions of AI Assistant behind a feature flag.