-
Notifications
You must be signed in to change notification settings - Fork 4.6k
🪟 🧪 [Experiment] Show source selector on signup form #18468
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
Changes from 4 commits
676ca38
fba2d5b
272e4dd
c8fc590
b591170
24b734c
5b8a4dc
193a644
01be2ac
6c39ed8
2de8b3c
163c411
c3edfb7
4127ba1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
import { useLocation } from "react-router-dom"; | ||
|
||
interface ILocationState<T> extends Omit<Location, "state"> { | ||
state: T; | ||
} | ||
|
||
export const useLocationState = <T>(): T => { | ||
const location = useLocation() as unknown as ILocationState<T>; | ||
return location.state; | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,197 @@ | ||
import { faPlus } from "@fortawesome/free-solid-svg-icons"; | ||
import { FontAwesomeIcon } from "@fortawesome/react-fontawesome"; | ||
import React, { useCallback, useMemo, useState } from "react"; | ||
import { FormattedMessage, useIntl } from "react-intl"; | ||
import { components } from "react-select"; | ||
import { MenuListProps } from "react-select"; | ||
|
||
import { GAIcon } from "components/icons/GAIcon"; | ||
import { ControlLabels } from "components/LabeledControl"; | ||
import { | ||
DropDown, | ||
DropDownOptionDataItem, | ||
DropDownOptionProps, | ||
OptionView, | ||
SingleValueIcon, | ||
SingleValueProps, | ||
SingleValueView, | ||
} from "components/ui/DropDown"; | ||
import { Text } from "components/ui/Text"; | ||
|
||
import { ReleaseStage } from "core/request/AirbyteClient"; | ||
import { useModalService } from "hooks/services/Modal"; | ||
import RequestConnectorModal from "views/Connector/RequestConnectorModal"; | ||
import styles from "views/Connector/ServiceForm/components/Controls/ConnectorServiceTypeControl/ConnectorServiceTypeControl.module.scss"; | ||
import { useAnalyticsTrackFunctions } from "views/Connector/ServiceForm/components/Controls/ConnectorServiceTypeControl/useAnalyticsTrackFunctions"; | ||
import { WarningMessage } from "views/Connector/ServiceForm/components/WarningMessage"; | ||
|
||
import { useGetSourceDefinitions } from "./useGetSourceDefinitions"; | ||
import { getSortedDropdownData } from "./utils"; | ||
|
||
// eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
type MenuWithRequestButtonProps = MenuListProps<DropDownOptionDataItem, false> & { selectProps: any }; | ||
|
||
const ConnectorList: React.FC<React.PropsWithChildren<MenuWithRequestButtonProps>> = ({ children, ...props }) => ( | ||
<> | ||
<components.MenuList {...props}>{children}</components.MenuList> | ||
<div className={styles.connectorListFooter}> | ||
<button | ||
className={styles.requestNewConnectorBtn} | ||
onClick={() => props.selectProps.selectProps.onOpenRequestConnectorModal(props.selectProps.inputValue)} | ||
> | ||
<FontAwesomeIcon icon={faPlus} /> | ||
<FormattedMessage id="connector.requestConnectorBlock" /> | ||
</button> | ||
</div> | ||
</> | ||
); | ||
|
||
const StageLabel: React.FC<{ releaseStage?: ReleaseStage }> = ({ releaseStage }) => { | ||
if (!releaseStage) { | ||
return null; | ||
} | ||
|
||
if (releaseStage === ReleaseStage.generally_available) { | ||
return <GAIcon />; | ||
} | ||
|
||
return ( | ||
<div className={styles.stageLabel}> | ||
<FormattedMessage id={`connector.releaseStage.${releaseStage}`} defaultMessage={releaseStage} /> | ||
</div> | ||
); | ||
}; | ||
|
||
const Option: React.FC<DropDownOptionProps> = (props) => { | ||
return ( | ||
<components.Option {...props}> | ||
<OptionView | ||
data-testid={props.data.label} | ||
isSelected={props.isSelected} | ||
isDisabled={props.isDisabled} | ||
isFocused={props.isFocused} | ||
> | ||
<div className={styles.connectorName}> | ||
{props.data.img || null} | ||
<Text size="lg">{props.label}</Text> | ||
</div> | ||
<StageLabel releaseStage={props.data.releaseStage} /> | ||
</OptionView> | ||
</components.Option> | ||
); | ||
}; | ||
|
||
// eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
const SingleValue: React.FC<SingleValueProps<any>> = (props) => { | ||
return ( | ||
<SingleValueView> | ||
{props.data.img && <SingleValueIcon>{props.data.img}</SingleValueIcon>} | ||
<div> | ||
<components.SingleValue className={styles.singleValueContent} {...props}> | ||
{props.data.label} | ||
<StageLabel releaseStage={props.data.releaseStage} /> | ||
</components.SingleValue> | ||
</div> | ||
</SingleValueView> | ||
); | ||
}; | ||
|
||
interface SignupSourceDropdownProps { | ||
disabled?: boolean; | ||
email: string; | ||
} | ||
|
||
export const SignupSourceDropdown: React.FC<SignupSourceDropdownProps> = ({ disabled, email }) => { | ||
const { formatMessage } = useIntl(); | ||
const { openModal, closeModal } = useModalService(); | ||
const { trackMenuOpen, trackNoOptionMessage, trackConnectorSelection } = useAnalyticsTrackFunctions("source"); | ||
|
||
const { data: availableSources } = useGetSourceDefinitions(); | ||
|
||
const [sourceDefinitionId, setSourceDefinitionId] = useState<string>(""); | ||
|
||
const onChangeServiceType = useCallback((sourceDefinitionId: string) => { | ||
setSourceDefinitionId(sourceDefinitionId); | ||
localStorage.setItem("exp-signup-selected-source-definition-id", sourceDefinitionId); | ||
}, []); | ||
|
||
const sortedDropDownData = useMemo(() => getSortedDropdownData(availableSources ?? []), [availableSources]); | ||
|
||
const getNoOptionsMessage = useCallback( | ||
({ inputValue }: { inputValue: string }) => { | ||
trackNoOptionMessage(inputValue); | ||
return formatMessage({ id: "form.noConnectorFound" }); | ||
}, | ||
[formatMessage, trackNoOptionMessage] | ||
); | ||
|
||
const selectedService = React.useMemo( | ||
() => sortedDropDownData.find((s) => s.value === sourceDefinitionId), | ||
[sourceDefinitionId, sortedDropDownData] | ||
); | ||
|
||
const handleSelect = useCallback( | ||
(item: DropDownOptionDataItem | null) => { | ||
if (item && onChangeServiceType) { | ||
onChangeServiceType(item.value); | ||
trackConnectorSelection(item.value, item.label || ""); | ||
} | ||
}, | ||
[onChangeServiceType, trackConnectorSelection] | ||
); | ||
|
||
const selectProps = useMemo( | ||
() => ({ | ||
onOpenRequestConnectorModal: (input: string) => | ||
openModal({ | ||
title: formatMessage({ id: "connector.requestConnector" }), | ||
content: () => ( | ||
<RequestConnectorModal | ||
connectorType="source" | ||
workspaceEmail={email} | ||
searchedConnectorName={input} | ||
onClose={closeModal} | ||
/> | ||
), | ||
}), | ||
}), | ||
[closeModal, formatMessage, openModal, email] | ||
); | ||
|
||
if (!Boolean(sortedDropDownData.length)) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Don't show the selector if we don't have any data to show, which will be weird, but just in case we can't fetch the |
||
return null; | ||
} | ||
return ( | ||
<> | ||
<ControlLabels | ||
label={formatMessage({ | ||
id: "login.sourceSelector", | ||
})} | ||
> | ||
<DropDown | ||
value={sourceDefinitionId} | ||
components={{ | ||
MenuList: ConnectorList, | ||
Option, | ||
SingleValue, | ||
}} | ||
selectProps={selectProps} | ||
isDisabled={disabled} | ||
isSearchable | ||
placeholder={formatMessage({ | ||
id: "form.selectConnector", | ||
})} | ||
options={sortedDropDownData} | ||
onChange={handleSelect} | ||
onMenuOpen={trackMenuOpen} | ||
noOptionsMessage={getNoOptionsMessage} | ||
data-testid="serviceType" | ||
/> | ||
</ControlLabels> | ||
{selectedService && | ||
(selectedService.releaseStage === ReleaseStage.alpha || selectedService.releaseStage === ReleaseStage.beta) && ( | ||
<WarningMessage stage={selectedService.releaseStage} /> | ||
)} | ||
</> | ||
); | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
export { SignupSourceDropdown } from "./SignupSourceDropdown"; |
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,30 @@ | ||
import { useQuery } from "react-query"; | ||
|
||
import { DestinationDefinitionRead, SourceDefinitionRead } from "core/request/AirbyteClient"; | ||
|
||
import availableSourceDefinitions from "./sourceDefinitions.json"; | ||
|
||
interface Catalog { | ||
destinations: DestinationDefinitionRead[]; | ||
sources: SourceDefinitionRead[]; | ||
} | ||
const fetchCatalog = async (): Promise<Catalog> => { | ||
const path = "https://storage.googleapis.com/prod-airbyte-cloud-connector-metadata-service/cloud_catalog.json"; | ||
const response = await fetch(path); | ||
return response.json(); | ||
}; | ||
export const useGetSourceDefinitions = () => { | ||
return useQuery<Catalog, Error, Catalog["sources"]>("cloud_catalog", fetchCatalog, { | ||
select: (data) => { | ||
return data.sources.map((source) => { | ||
letiescanciano marked this conversation as resolved.
Show resolved
Hide resolved
|
||
const icon = availableSourceDefinitions.sourceDefinitions.find( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since the cloud_catalog does not include icons, I have a hardcoded I think this is good enough for the experiment There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Given that we know, that this won't be the final solution here, and we already merge from the catalog directly from the googleapis plus your local one, I'd suggest we just remove the call to the cloud_catalog.json altogether and for the time being while this experiment runs just use an embedded JSON file instead. Given we anyway need to rework that for production use I feel it's easier (plus faster, since we don't need to do the additional request on the sign-up page). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The benefit that I see on keeping it, is that we'll get the more recent cloud catalog. If we embed the json, we'll miss that. If that's ok (I'm not sure how often connectors change) I agree on having just the json file. |
||
(src) => src.sourceDefinitionId === source.sourceDefinitionId | ||
)?.icon; | ||
return { | ||
...source, | ||
icon, | ||
}; | ||
}); | ||
}, | ||
}); | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,43 @@ | ||
import { ConnectorIcon } from "components/ConnectorIcon"; | ||
|
||
import { Connector } from "core/domain/connector"; | ||
import { ReleaseStage, SourceDefinitionRead } from "core/request/AirbyteClient"; | ||
import { naturalComparator } from "utils/objects"; | ||
|
||
/** | ||
* Returns the order for a specific release stage label. This will define | ||
* in what order the different release stages are shown inside the select. | ||
* They will be shown in an increasing order (i.e. 0 on top) | ||
*/ | ||
const getOrderForReleaseStage = (stage?: ReleaseStage): number => { | ||
switch (stage) { | ||
case ReleaseStage.beta: | ||
return 1; | ||
case ReleaseStage.alpha: | ||
return 2; | ||
default: | ||
return 0; | ||
} | ||
}; | ||
interface ServiceDropdownOption { | ||
label: string; | ||
value: string; | ||
img: JSX.Element; | ||
releaseStage: ReleaseStage | undefined; | ||
} | ||
const transformConnectorDefinitionToDropdownOption = (item: SourceDefinitionRead): ServiceDropdownOption => ({ | ||
label: item.name, | ||
value: Connector.id(item), | ||
img: <ConnectorIcon icon={item.icon} />, | ||
releaseStage: item.releaseStage, | ||
}); | ||
|
||
const sortByReleaseStage = (a: ServiceDropdownOption, b: ServiceDropdownOption) => { | ||
if (a.releaseStage !== b.releaseStage) { | ||
return getOrderForReleaseStage(a.releaseStage) - getOrderForReleaseStage(b.releaseStage); | ||
} | ||
return naturalComparator(a.label, b.label); | ||
}; | ||
|
||
export const getSortedDropdownData = (availableConnectorDefinitions: SourceDefinitionRead[]): ServiceDropdownOption[] => | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We only sort by release stage, but if we want to replicate There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think that's fine for now. If we'd keep this feature, we'd anyway need to remove this second component again, so I don't think we need to have the full functionality for now. |
||
availableConnectorDefinitions.map(transformConnectorDefinitionToDropdownOption).sort(sortByReleaseStage); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
export const EXP_SOURCE_SIGNUP_SELECTOR = "exp-signup-selected-source-definition-id"; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,12 +1,24 @@ | ||
import { useEffect } from "react"; | ||
import { Navigate } from "react-router-dom"; | ||
|
||
import { useExperiment } from "hooks/services/Experiment"; | ||
|
||
import { RoutePaths } from "../../../pages/routePaths"; | ||
import { CloudRoutes } from "../cloudRoutes"; | ||
import { EXP_SOURCE_SIGNUP_SELECTOR } from "../components/experiments/constants"; | ||
import { useListCloudWorkspaces } from "../services/workspaces/CloudWorkspacesService"; | ||
|
||
export const DefaultView: React.FC = () => { | ||
const workspaces = useListCloudWorkspaces(); | ||
// exp-signup-selected-source-definition | ||
const isSignupSourceSelectorExperiment = useExperiment("authPage.signup.sourceSelector", false); | ||
const sourceDefinitionId = localStorage.getItem(EXP_SOURCE_SIGNUP_SELECTOR); | ||
|
||
useEffect(() => { | ||
setTimeout(() => { | ||
localStorage.removeItem(EXP_SOURCE_SIGNUP_SELECTOR); | ||
letiescanciano marked this conversation as resolved.
Show resolved
Hide resolved
|
||
}, 5000); | ||
}, []); | ||
// Only show the workspace creation list if there is more than one workspace | ||
// otherwise redirect to the single workspace | ||
return ( | ||
|
@@ -17,6 +29,11 @@ export const DefaultView: React.FC = () => { | |
: `/${RoutePaths.Workspaces}/${workspaces[0].workspaceId}` | ||
} | ||
replace | ||
// exp-signup-selected-source-definition | ||
{...(isSignupSourceSelectorExperiment && { | ||
state: { sourceDefinitionId }, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I feel we might want to more often navigate to the new-source (or destination) page in the future with preselecting a connector, I feel it would be nice to have this option as an actual query parameter (not as a react router state), so that we could actually also craft those URLs manually and link to them. We could also merge that logic separate from this PR, since I feel this can stay inside without any link to this specific experiment. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think that would be nice if you feel that this can be useful in the future. I think that could be a separated issue (and as you are saying a different PR) and not part of this experiment. |
||
to: `/${RoutePaths.Workspaces}/${workspaces[0].workspaceId}/${RoutePaths.Source}/${RoutePaths.SourceNew}`, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we lower the UX for new signups significantly with this change. So far users usually run through the New connection or onboarding setup, which basically allows them to setup the source, then destination, then connection, i.e. they end their first flow with a fully setup connection. With this change they'll be navigated to the new source setup page and once they setup the source, will just be on this source's detail page, and basically now anyhow in a flow to setup a destination or create a connection. I think this will decrease our setup experience significantly, and don't think this is a change we should do. Is there a specific reason we would not want to throw them into the new connection creation flow here and preselect the source? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That's a great point actually. I agree it makes more sense to direct them to the Source Setup step on the onboarding flow. I will make the changes. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd recommend you throw them into connection setup and not the onboarding flow. We might remove the onboarding page before this experiment here is done, given the low effect we see of it. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @timroes this is now updated 💪🏼 https://www.loom.com/share/2f34ee77a49348c3950988b72da02e55 |
||
})} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. adding |
||
/> | ||
); | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -9,6 +9,7 @@ import { LabeledInput, Link } from "components"; | |
import { Button } from "components/ui/Button"; | ||
|
||
import { useExperiment } from "hooks/services/Experiment"; | ||
import { SignupSourceDropdown } from "packages/cloud/components/experiments/SignupSourceDropdown"; | ||
import { FieldError } from "packages/cloud/lib/errors/FieldError"; | ||
import { useAuthService } from "packages/cloud/services/auth/AuthService"; | ||
import { isGdprCountry } from "utils/dataPrivacy"; | ||
|
@@ -180,6 +181,7 @@ export const SignupForm: React.FC = () => { | |
|
||
const showName = !useExperiment("authPage.signup.hideName", false); | ||
const showCompanyName = !useExperiment("authPage.signup.hideCompanyName", false); | ||
const showSourceSelector = useExperiment("authPage.signup.sourceSelector", false); | ||
|
||
const validationSchema = useMemo(() => { | ||
const shape = { | ||
|
@@ -223,7 +225,7 @@ export const SignupForm: React.FC = () => { | |
validateOnBlur | ||
validateOnChange | ||
> | ||
{({ isValid, isSubmitting, status }) => ( | ||
{({ isValid, isSubmitting, status, values }) => ( | ||
<Form> | ||
{(showName || showCompanyName) && ( | ||
<RowFieldItem> | ||
|
@@ -232,6 +234,12 @@ export const SignupForm: React.FC = () => { | |
</RowFieldItem> | ||
)} | ||
|
||
{/* exp-select-source-signup */} | ||
{showSourceSelector && ( | ||
<FieldItem> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Once not an experiment anymore, this value should properly be tracked via Formik state and handled only in the |
||
<SignupSourceDropdown disabled={isSubmitting} email={values.email} /> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. New field! |
||
</FieldItem> | ||
)} | ||
<FieldItem> | ||
<EmailField /> | ||
</FieldItem> | ||
|
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 duplicated
<ConnectorServiceTypeControl/>
. If we decide to keep this, all these components can be extracted and reused between both components