Skip to content

OINT-1289: WIP bringing back modal add #561

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

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
182 changes: 166 additions & 16 deletions apps/web/app/connect/AddConnection.client.tsx
Original file line number Diff line number Diff line change
@@ -1,18 +1,139 @@
'use client'

import type {ReactElement} from 'react'
import type {ConnectorConfig} from '@openint/api-v1/trpc/routers/connectorConfig.models'
import type {Id} from '@openint/cdk'

import {SearchIcon} from 'lucide-react'
import {useState} from 'react'
import {type ConnectorName} from '@openint/api-v1/trpc/routers/connector.models'
import {Label} from '@openint/shadcn/ui'
import {ConnectorConfigCard} from '@openint/ui-v1/domain-components/ConnectorConfigCard'
import {cn} from '@openint/shadcn/lib/utils'
import {
Dialog,
DialogContent,
DialogHeader,
DialogTitle,
Input,
} from '@openint/shadcn/ui'
import {ConnectorDisplay} from '@openint/ui-v1/domain-components/ConnectorDisplay'
import {useMutableSearchParams} from '@openint/ui-v1/hooks/useStateFromSearchParam'
import {ConnectorConnectContainer} from './ConnectorConnect.client'

export type ConnectorConfigForCustomer = Pick<
ConnectorConfig<'connector'>,
'id' | 'connector_name' | 'connector'
>

export type AddConnectionPrefetchedCard = {
connectorName: ConnectorName
connectorDisplayName?: string
card: ReactElement
}

export function AddConnectionClient({
cards,
className,
}: {
cards: AddConnectionPrefetchedCard[]
className?: string
}) {
const [isOpen, setIsOpen] = useState(true)
const [_, setSearchParams] = useMutableSearchParams()
Comment on lines +40 to +41
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

logic: isOpen is initialized to true but there's no way to reopen the modal if closed. Consider making this controlled by parent component.


const onClose = () => {
setIsOpen(false)
setSearchParams({view: 'manage'}, {shallow: false})
}
const [searchQuery, setSearchQuery] = useState('')
const [isFocused, setIsFocused] = useState(false)

const filteredCards = cards.filter((card) => {
const searchLower = searchQuery.toLowerCase()
return (
card.connectorName.toLowerCase().includes(searchLower) ||
card.connectorDisplayName?.toLowerCase().includes(searchLower)
)
})

return (
<Dialog open={isOpen} onOpenChange={(open) => !open && onClose()}>
{/* TODO: consider making full screen horizontal on mobile https://github.com/radix-ui/themes/issues/142 */}
<DialogContent
className={cn(
'flex h-[85vh] max-h-[600px] max-w-2xl flex-col gap-0 overflow-hidden p-0 shadow-xl',
className,
)}>
<DialogHeader className="bg-foreground/5 flex-shrink-0 border-b px-6 py-4">
<DialogTitle className="text-md">Add Integrations</DialogTitle>
</DialogHeader>

<div className="flex flex-1 flex-col overflow-hidden">
<div
className={cn(
'bg-background sticky top-0 z-10 flex-shrink-0 transition-all duration-200',
'border-b border-b-transparent',
isFocused ? 'border-b-gray-100 shadow-sm' : '',
)}>
<div className="p-6 pb-3">
<div className="relative">
<SearchIcon className="absolute left-3 top-1/2 h-4 w-4 -translate-y-1/2 transform text-gray-500" />
<Input
placeholder="Search integrations"
value={searchQuery}
onChange={(e) => setSearchQuery(e.target.value)}
className="border-gray-200 bg-gray-50/50 pl-10 transition-all duration-200 focus:bg-white focus:shadow-sm"
onFocus={() => setIsFocused(true)}
onBlur={() => setIsFocused(false)}
/>
</div>
</div>
</div>

<div
className="relative flex-1 overflow-y-auto"
onScroll={(e) => {
const scrollTop = e.currentTarget.scrollTop
const shouldBeFocused = scrollTop > 5
if (shouldBeFocused !== isFocused) {
setIsFocused(shouldBeFocused)
}
}}>
{/* Mobile view */}
<div className="p-6 pt-3 md:hidden">
{filteredCards.map((card, index) => (
<div key={`${card.connectorName}-${index}`}>
Comment on lines +103 to +104
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

style: Using array index as part of key prop could cause issues with React's reconciliation if cards are reordered

Suggested change
{filteredCards.map((card, index) => (
<div key={`${card.connectorName}-${index}`}>
{filteredCards.map((card) => (
<div key={card.connectorName}>

{card.card}
{index < filteredCards.length - 1 && (
<div className="my-1 h-px bg-gray-100" />
)}
</div>
))}
</div>

{/* Desktop view: Grid layout */}
<div className="hidden p-6 pt-3 md:grid md:grid-cols-2 md:gap-4">
{filteredCards.map((card, index) => (
<div
key={`${card.connectorName}-${index}`}
className="transition-transform duration-100 hover:scale-[1.01]">
{card.card}
</div>
))}
</div>

{filteredCards.length === 0 && (
<div className="col-span-2 py-16 text-center text-gray-500">
No integrations found matching{' '}
{searchQuery ? `"${searchQuery}"` : 'your search'}
</div>
)}
</div>
</div>
</DialogContent>
</Dialog>
)
}

export function AddConnectionCard({
connectorConfig,
}: {
Expand All @@ -25,20 +146,49 @@ export function AddConnectionCard({
connector={connectorConfig.connector!}
connectorConfigId={connectorConfig.id as Id['ccfg']}>
{({isConnecting, handleConnect}) => (
<ConnectorConfigCard
displayNameLocation="right"
// TODO: fix this
connectorConfig={connectorConfig as ConnectorConfig<'connector'>}
onPress={() => {
if (isConnecting) {
return
}
handleConnect()
}}>
<Label className="text-muted-foreground pointer-events-none ml-auto text-sm">
{isConnecting ? 'Connecting...' : 'Connect'}
</Label>
</ConnectorConfigCard>
<>
{/* NOTE/TODO: note sure how to remove duplication,
its there as we're using tailwind for consistent breakpoints with AddConnectionClient */}
Comment on lines +150 to +151
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

style: Consider extracting common ConnectorDisplay logic into a shared component to avoid duplication between mobile and desktop views

{/* Mobile view (visible below md breakpoint) */}
<div className="md:hidden">
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider extracting the duplicated rendering logic for ConnectorDisplay in the mobile (lines 150–170) and desktop (lines 172–190) views into a shared component.

<ConnectorDisplay
connector={connectorConfig.connector!}
className={cn(
'transition-transform duration-100',
!isConnecting && 'hover:scale-[1.01]',
isConnecting && 'cursor-not-allowed opacity-70',
)}
displayBadges={false}
mode="row"
onPress={() => {
if (isConnecting) {
return
}
handleConnect()
}}
/>
</div>

{/* Desktop view (visible at md breakpoint and above) */}
<div className="hidden md:block">
<ConnectorDisplay
connector={connectorConfig.connector!}
className={cn(
'transition-transform duration-100',
!isConnecting && 'hover:scale-[1.01]',
isConnecting && 'cursor-not-allowed opacity-70',
)}
displayBadges={false}
mode="card"
onPress={() => {
if (isConnecting) {
return
}
handleConnect()
}}
/>
</div>
</>
)}
</ConnectorConnectContainer>
)
Expand Down
148 changes: 90 additions & 58 deletions apps/web/app/connect/MyConnections.client.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
'use client'

import {useSuspenseQuery} from '@tanstack/react-query'
import {Link} from 'lucide-react'
import React from 'react'
import {type ConnectorName} from '@openint/api-v1/trpc/routers/connector.models'
import {Id} from '@openint/cdk'
Expand All @@ -9,7 +10,7 @@ import {
CommandPopover,
ConnectionStatusPill,
DataTileView,
Spinner,
LoadingSpinner,
useMutableSearchParams,
} from '@openint/ui-v1'
import {ConnectionCard} from '@openint/ui-v1/domain-components/ConnectionCard'
Expand Down Expand Up @@ -41,75 +42,106 @@ export function MyConnectionsClient(props: {
const definitions = useCommandDefinitionMap()

if (isLoading) {
return (
<div className="flex min-h-[200px] items-center justify-center">
<Spinner />
</div>
)
return <LoadingSpinner />
}

if (!res.data?.items?.length || res.data.items.length === 0) {
return (
<div className="flex flex-col items-center gap-4 py-8 text-center">
<p className="text-muted-foreground">
You have no configured integrations.
</p>
<div className="flex min-h-[200px] flex-col items-center justify-center gap-6 py-12">
<div className="flex flex-col items-center gap-4 text-center">
<div className="bg-primary/10 rounded-full p-4">
<Link className="text-primary h-8 w-8" />
</div>
<h3 className="text-xl font-semibold">
Let&apos;s Get You Connected
</h3>
<p className="text-muted-foreground">
You have not configured any integrations yet. Connect your first app
to get started.
</p>
</div>
<Button
variant="default"
onClick={() => setSearchParams({view: 'add'}, {shallow: true})}>
Add your first integration
size="lg"
className="font-medium"
onClick={() => {
setIsLoading(true)
setSearchParams({view: 'add'}, {shallow: false})
}}
Comment on lines +67 to +69
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

style: Setting loading state before navigation is unnecessary since component will unmount. Remove setIsLoading(true).

Suggested change
setIsLoading(true)
setSearchParams({view: 'add'}, {shallow: false})
}}
setSearchParams({view: 'add'}, {shallow: false})
}}

disabled={isLoading}>
Add First Integration
</Button>
Comment on lines 63 to 72
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: Declare and initialize the isLoading state to avoid runtime errors. [possible bug]

Suggested change
<Button
variant="default"
onClick={() => setSearchParams({view: 'add'}, {shallow: true})}>
Add your first integration
size="lg"
className="font-medium"
onClick={() => {
setIsLoading(true)
setSearchParams({view: 'add'}, {shallow: false})
}}
disabled={isLoading}>
Add First Integration
</Button>
const [isLoading, setIsLoading] = useState(false)
...
<Button
size="lg"
className="font-medium"
onClick={() => {
setIsLoading(true)
setSearchParams({ view: 'add' }, { shallow: false })
}}
disabled={isLoading}>
Add First Integration
</Button>

</div>
)
}

return (
<DataTileView
data={res.data.items}
columns={[]}
getItemId={(conn) => conn.id}
renderItem={(conn) => {
const renderCard = ({handleConnect}: {handleConnect?: () => void}) => (
<ConnectionCard
connection={conn}
className="relative"
onReconnect={handleConnect}>
<CommandPopover
className="absolute right-2 top-2"
hideGroupHeadings
initialParams={{
connection_id: conn.id,
}}
ctx={{}}
definitions={definitions}
header={
<>
<div className="flex items-center justify-center gap-1 text-center">
<ConnectionStatusPill status={conn.status} />
<span className="text-muted-foreground text-xs">
({timeSince(conn.updated_at)})
</span>
</div>
</>
}
/>
</ConnectionCard>
)
<>
<div className="flex flex-col items-start justify-between sm:flex-row sm:items-center">
<h1 className="text-xl font-bold">My Integrations</h1>
<Button
size="lg"
onClick={() => {
setIsLoading(true)
setSearchParams({view: 'add'}, {shallow: false})
}}
disabled={isLoading}>
Add Integration
</Button>
</div>

<div className="mt-4"></div>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

style: Empty div with mt-4 can be replaced by adding margin to DataTileView component.

<DataTileView
data={res.data.items}
columns={[]}
className="grid grid-cols-1 gap-6 sm:grid-cols-3 md:grid-cols-4"
getItemId={(conn) => conn.id}
renderItem={(conn) => {
const renderCard = ({
handleConnect,
}: {
handleConnect?: () => void
}) => (
<ConnectionCard
connection={conn}
className="relative"
onReconnect={handleConnect}>
<CommandPopover
className="absolute right-2 top-2"
hideGroupHeadings
initialParams={{
connection_id: conn.id,
}}
ctx={{}}
definitions={definitions}
header={
<>
<div className="flex items-center justify-center gap-1 text-center">
<ConnectionStatusPill status={conn.status} />
<span className="text-muted-foreground text-xs">
({timeSince(conn.updated_at)})
</span>
</div>
</>
}
/>
</ConnectionCard>
)

if (conn.status !== 'disconnected') {
return renderCard({handleConnect: undefined})
}
if (conn.status !== 'disconnected') {
return renderCard({handleConnect: undefined})
}

return (
<ConnectorConnectContainer
connectorName={conn.connector_name as ConnectorName}
connector={{auth_type: 'OAUTH2'} as any}
connectorConfigId={conn.connector_config_id as Id['ccfg']}
connectionId={conn.id as Id['conn']}>
{renderCard}
</ConnectorConnectContainer>
)
}}
/>
return (
<ConnectorConnectContainer
connectorName={conn.connector_name as ConnectorName}
connector={{auth_type: 'OAUTH2'} as any}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

logic: Hardcoding auth_type as 'OAUTH2' and using type assertion is unsafe. Should handle different auth types properly.

connectorConfigId={conn.connector_config_id as Id['ccfg']}
connectionId={conn.id as Id['conn']}>
{renderCard}
</ConnectorConnectContainer>
)
}}
/>
</>
)
}
Loading