diff --git a/apps/studio/components/interfaces/Database/Replication/DestinationPanel/DestinationForm/AnalyticsBucket/Fields.tsx b/apps/studio/components/interfaces/Database/Replication/DestinationPanel/DestinationForm/AnalyticsBucket/Fields.tsx index 4c66ffe3352..06943c0636f 100644 --- a/apps/studio/components/interfaces/Database/Replication/DestinationPanel/DestinationForm/AnalyticsBucket/Fields.tsx +++ b/apps/studio/components/interfaces/Database/Replication/DestinationPanel/DestinationForm/AnalyticsBucket/Fields.tsx @@ -1,5 +1,5 @@ import { useParams } from 'common' -import { Eye, EyeOff, Loader2 } from 'lucide-react' +import { Eye, EyeOff } from 'lucide-react' import { useState } from 'react' import { useWatch, type UseFormReturn } from 'react-hook-form' import { @@ -13,11 +13,11 @@ import { SelectItem, SelectSeparator, SelectTrigger, - WarningIcon, } from 'ui' import { Admonition } from 'ui-patterns/Admonition' import { Input as PasswordInput } from 'ui-patterns/DataInputs/Input' import { FormItemLayout } from 'ui-patterns/form/FormItemLayout/FormItemLayout' +import { SelectionListState } from 'ui-patterns/SelectionListState' import { CREATE_NEW_KEY, @@ -25,6 +25,11 @@ import { STORED_SECRET_PLACEHOLDER, } from '../DestinationForm.constants' import type { DestinationPanelSchemaType } from '../DestinationForm.schema' +import { + isMetadataListErrorVisible, + isMetadataListLoading, + useRefreshOnOpen, +} from '../useRefreshOnOpen' import { InlineLink } from '@/components/ui/InlineLink' import { useAnalyticsBucketsQuery } from '@/data/storage/analytics-buckets-query' import { useIcebergNamespacesQuery } from '@/data/storage/iceberg-namespaces-query' @@ -52,6 +57,19 @@ const getS3AccessKeyTriggerLabel = ({ return value } +const getNamespaceTriggerLabel = ({ + canSelectNamespace, + value, +}: { + canSelectNamespace: boolean + value: string | undefined +}) => { + if (!canSelectNamespace) return 'Select a bucket first' + if (value === CREATE_NEW_NAMESPACE) return 'Create a new namespace' + + return value || 'Select a namespace' +} + export const AnalyticsBucketFields = ({ form, editMode, @@ -73,10 +91,14 @@ export const AnalyticsBucketFields = ({ const { data: keysData, isSuccess: isSuccessKeys, - isPending: isLoadingKeys, + isPending: isPendingKeys, + isFetching: isFetchingKeys, isError: isErrorKeys, + refetch: refetchKeys, } = useStorageCredentialsQuery({ projectRef }) const s3Keys = keysData?.data ?? [] + const isLoadingKeys = isMetadataListLoading(isPendingKeys || isFetchingKeys, s3Keys.length) + const isKeysErrorVisible = isMetadataListErrorVisible(isErrorKeys, s3Keys.length) const keyNoLongerExists = (s3AccessKeyId ?? '').length > 0 && s3AccessKeyId !== CREATE_NEW_KEY && @@ -84,20 +106,44 @@ export const AnalyticsBucketFields = ({ const { data: analyticsBuckets = [], - isPending: isLoadingBuckets, + isPending: isPendingBuckets, + isFetching: isFetchingBuckets, isError: isErrorBuckets, + refetch: refetchBuckets, } = useAnalyticsBucketsQuery({ projectRef }) + const isLoadingBuckets = isMetadataListLoading( + isPendingBuckets || isFetchingBuckets, + analyticsBuckets.length + ) + const isBucketsErrorVisible = isMetadataListErrorVisible(isErrorBuckets, analyticsBuckets.length) const canSelectNamespace = !!warehouseName const { data: namespaces = [], - isPending: isLoadingNamespaces, + isPending: isPendingNamespaces, + isFetching: isFetchingNamespaces, isError: isErrorNamespaces, + refetch: refetchNamespaces, } = useIcebergNamespacesQuery( { projectRef, warehouse: warehouseName }, { enabled: !!warehouseName } ) + const isLoadingNamespaces = isMetadataListLoading( + isPendingNamespaces || isFetchingNamespaces, + namespaces.length + ) + const isNamespacesErrorVisible = isMetadataListErrorVisible(isErrorNamespaces, namespaces.length) + const { handleOpenChange: handleRefreshBucketsOnOpen } = useRefreshOnOpen({ + refetch: refetchBuckets, + }) + const { handleOpenChange: handleRefreshNamespacesOnOpen } = useRefreshOnOpen({ + isEnabled: canSelectNamespace, + refetch: refetchNamespaces, + }) + const { handleOpenChange: handleRefreshKeysOnOpen } = useRefreshOnOpen({ + refetch: refetchKeys, + }) return (
@@ -113,61 +159,45 @@ export const AnalyticsBucketFields = ({ layout="horizontal" description="The Analytics Bucket where data will be stored" > - {isLoadingBuckets ? ( -
} > - {isLoadingKeys ? ( - - ) - } - if (isErrorProjects) { - return ( - - ) - } return ( - {projectLabel(value) ?? placeholder} - {projects.length === 0 ? ( - - No active projects available + + {projects.map((project) => ( + +
+ {project.name} + + {project.ref} · {project.region} + +
- ) : ( - projects.map((project) => ( - -
- {project.name} - - {project.ref} · {project.region} - -
- - )) - )} + ))} @@ -785,8 +779,10 @@ const BucketSelection = ({ const { data: bucketsData, - isPending: isLoadingBuckets, + isPending: isPendingBuckets, + isFetching: isFetchingBuckets, isError: isErrorBuckets, + refetch: refetchBuckets, } = usePaginatedBucketsQuery( { projectRef: ducklakeStorageProjectRef }, { enabled: !!ducklakeStorageProjectRef } @@ -799,44 +795,24 @@ const BucketSelection = ({ ), [bucketsData] ) + const isBucketsErrorVisible = isMetadataListErrorVisible(isErrorBuckets, buckets.length) + const { handleOpenChange: handleRefreshBucketsOnOpen } = useRefreshOnOpen({ + isEnabled: !!ducklakeStorageProjectRef, + refetch: refetchBuckets, + }) if (!ducklakeStorageProjectRef) { return ( - - ) - } - if (isLoadingBuckets) { - return ( - - ) - } - if (isErrorBuckets) { - return ( - + ) } return ( diff --git a/apps/studio/components/interfaces/Database/Replication/DestinationPanel/DestinationForm/useRefreshOnOpen.test.ts b/apps/studio/components/interfaces/Database/Replication/DestinationPanel/DestinationForm/useRefreshOnOpen.test.ts new file mode 100644 index 00000000000..d3577c58129 --- /dev/null +++ b/apps/studio/components/interfaces/Database/Replication/DestinationPanel/DestinationForm/useRefreshOnOpen.test.ts @@ -0,0 +1,27 @@ +import { describe, expect, it } from 'vitest' + +import { + isMetadataListErrorVisible, + isMetadataListLoading, + isMetadataValueLoading, +} from './useRefreshOnOpen' + +describe('replication metadata loading UI', () => { + it('shows a list skeleton only while pending or fetching an empty list', () => { + expect(isMetadataListLoading(true, 0)).toBe(true) + expect(isMetadataListLoading(true, 3)).toBe(false) + expect(isMetadataListLoading(false, 0)).toBe(false) + }) + + it('shows a list error only when the request failed and the list is empty', () => { + expect(isMetadataListErrorVisible(true, 0)).toBe(true) + expect(isMetadataListErrorVisible(true, 3)).toBe(false) + expect(isMetadataListErrorVisible(false, 0)).toBe(false) + }) + + it('shows a record skeleton only while fetching a missing value', () => { + expect(isMetadataValueLoading(true, undefined)).toBe(true) + expect(isMetadataValueLoading(true, { name: 'analytics' })).toBe(false) + expect(isMetadataValueLoading(false, undefined)).toBe(false) + }) +}) diff --git a/apps/studio/components/interfaces/Database/Replication/DestinationPanel/DestinationForm/useRefreshOnOpen.ts b/apps/studio/components/interfaces/Database/Replication/DestinationPanel/DestinationForm/useRefreshOnOpen.ts new file mode 100644 index 00000000000..3d56091aaed --- /dev/null +++ b/apps/studio/components/interfaces/Database/Replication/DestinationPanel/DestinationForm/useRefreshOnOpen.ts @@ -0,0 +1,32 @@ +// Replication metadata (publication names, publication tables, source tables, +// columns) and similar destination-form option lists follow one rule: +// +// - Opening a picker always refetches. +// - Loading and error UI only appear when there is nothing to show, so a +// background refresh never replaces existing options with a skeleton. +// +// Pass `isPending || isFetching` into the loading helpers. `isPending` covers +// "query not started yet" (including while a parent id is still loading); +// `isFetching` covers an in-flight request. A populated list stays visible. + +export const isMetadataListLoading = (isPendingOrFetching: boolean, itemCount: number) => + isPendingOrFetching && itemCount === 0 + +export const isMetadataListErrorVisible = (isError: boolean, itemCount: number) => + isError && itemCount === 0 + +export const isMetadataValueLoading = (isPendingOrFetching: boolean, value: unknown) => + isPendingOrFetching && value == null + +interface UseRefreshOnOpenProps { + isEnabled?: boolean + refetch: () => unknown +} + +export const useRefreshOnOpen = ({ isEnabled = true, refetch }: UseRefreshOnOpenProps) => { + const handleOpenChange = (isOpen: boolean) => { + if (isOpen && isEnabled) void refetch() + } + + return { handleOpenChange } +} diff --git a/apps/studio/components/interfaces/Organization/BillingSettings/BillingEmail.test.tsx b/apps/studio/components/interfaces/Organization/BillingSettings/BillingEmail.test.tsx index 83f8d59289d..a5692cdce14 100644 --- a/apps/studio/components/interfaces/Organization/BillingSettings/BillingEmail.test.tsx +++ b/apps/studio/components/interfaces/Organization/BillingSettings/BillingEmail.test.tsx @@ -72,7 +72,13 @@ const mockUpdateCustomerProfile = () => { } const addRecipient = async (email: string) => { - const input = screen.getByPlaceholderText('Add additional recipients') + const input = screen + .getAllByRole('combobox') + .find((element): element is HTMLInputElement => element instanceof HTMLInputElement) + + expect(input).toBeDefined() + if (!input) return + await userEvent.click(input) await waitFor(() => expect(input).toHaveAttribute('aria-expanded', 'true')) // fireEvent.change (rather than userEvent.type) avoids racing the popover's open-state diff --git a/packages/ui-patterns/package.json b/packages/ui-patterns/package.json index 8ecf537b45c..9acb4eed9a0 100644 --- a/packages/ui-patterns/package.json +++ b/packages/ui-patterns/package.json @@ -582,6 +582,14 @@ "import": "./src/Row/index.tsx", "types": "./src/Row/index.tsx" }, + "./SelectionListState/SelectionListState": { + "import": "./src/SelectionListState/SelectionListState.tsx", + "types": "./src/SelectionListState/SelectionListState.tsx" + }, + "./SelectionListState": { + "import": "./src/SelectionListState/index.ts", + "types": "./src/SelectionListState/index.ts" + }, "./ShimmeringLoader/index.css": { "import": "./src/ShimmeringLoader/index.css", "types": "./src/ShimmeringLoader/index.css" diff --git a/packages/ui-patterns/src/SelectionListState/SelectionListState.test.tsx b/packages/ui-patterns/src/SelectionListState/SelectionListState.test.tsx new file mode 100644 index 00000000000..2cac72c194c --- /dev/null +++ b/packages/ui-patterns/src/SelectionListState/SelectionListState.test.tsx @@ -0,0 +1,22 @@ +import { render, screen } from '@testing-library/react' +import { describe, expect, it } from 'vitest' + +import { SelectionListState } from './SelectionListState' + +describe('SelectionListState', () => { + it('keeps one persistent live region while its state changes', () => { + const { container, rerender } = render() + + const liveRegion = container.querySelector('[aria-live="polite"]') + expect(liveRegion).not.toBeNull() + if (!liveRegion) throw new Error('Expected an aria-live status region') + expect(liveRegion).toHaveAttribute('aria-live', 'polite') + expect(liveRegion).toHaveClass('sr-only') + expect(liveRegion).toHaveTextContent('Loading options') + + rerender() + + expect(screen.getByText('No columns found')).toBe(liveRegion) + expect(liveRegion).not.toHaveClass('sr-only') + }) +}) diff --git a/packages/ui-patterns/src/SelectionListState/SelectionListState.tsx b/packages/ui-patterns/src/SelectionListState/SelectionListState.tsx new file mode 100644 index 00000000000..37eeaf58275 --- /dev/null +++ b/packages/ui-patterns/src/SelectionListState/SelectionListState.tsx @@ -0,0 +1,49 @@ +import { cn } from 'ui' + +import { GenericSelectionSkeletonLoader } from '../ShimmeringLoader' + +interface SelectionListStateProps { + className?: string + emptyLabel?: string + errorLabel?: string + isEmpty?: boolean + isError?: boolean + isLoading?: boolean + skeletonVariant?: 'command' | 'multi-select' | 'select' +} + +export const SelectionListState = ({ + className, + emptyLabel = 'No options available', + errorLabel = 'Unable to load options', + isEmpty = false, + isError = false, + isLoading = false, + skeletonVariant = 'select', +}: SelectionListStateProps) => { + let statusLabel: string | undefined + if (isLoading) statusLabel = 'Loading options' + else if (isError) statusLabel = errorLabel + else if (isEmpty) statusLabel = emptyLabel + + return ( + <> + {isLoading && ( + + )} +
+ {statusLabel} +
+ + ) +} diff --git a/packages/ui-patterns/src/SelectionListState/index.ts b/packages/ui-patterns/src/SelectionListState/index.ts new file mode 100644 index 00000000000..58af712bc12 --- /dev/null +++ b/packages/ui-patterns/src/SelectionListState/index.ts @@ -0,0 +1 @@ +export * from './SelectionListState' diff --git a/packages/ui-patterns/src/ShimmeringLoader/index.css b/packages/ui-patterns/src/ShimmeringLoader/index.css index d538577cbf8..00ca24a4462 100644 --- a/packages/ui-patterns/src/ShimmeringLoader/index.css +++ b/packages/ui-patterns/src/ShimmeringLoader/index.css @@ -29,3 +29,10 @@ background-position: 1000px 0; } } + +@media (prefers-reduced-motion: reduce) { + .shimmering-loader, + .dark .shimmering-loader { + animation: none; + } +} diff --git a/packages/ui-patterns/src/ShimmeringLoader/index.tsx b/packages/ui-patterns/src/ShimmeringLoader/index.tsx index 59d229a8f22..02192b94650 100644 --- a/packages/ui-patterns/src/ShimmeringLoader/index.tsx +++ b/packages/ui-patterns/src/ShimmeringLoader/index.tsx @@ -37,6 +37,42 @@ export const GenericSkeletonLoader = ({ className }: GenericSkeletonLoaderProps) ) +interface GenericSelectionSkeletonLoaderProps extends GenericSkeletonLoaderProps { + // Selection primitives have different row indicators: command lists have none, Select uses + // radio circles, and MultiSelector uses checkboxes. + variant?: 'command' | 'multi-select' | 'select' +} + +export const GenericSelectionSkeletonLoader = ({ + className, + variant = 'multi-select', +}: GenericSelectionSkeletonLoaderProps) => { + const hasIndicator = variant !== 'command' + const isSelect = variant === 'select' + + return ( + + ) +} + export const GenericTableLoader = ({ headers = [], numRows = 3, diff --git a/packages/ui-patterns/src/multi-select/multi-select.test.tsx b/packages/ui-patterns/src/multi-select/multi-select.test.tsx index 6fa101b670d..5dfed06154b 100644 --- a/packages/ui-patterns/src/multi-select/multi-select.test.tsx +++ b/packages/ui-patterns/src/multi-select/multi-select.test.tsx @@ -61,11 +61,13 @@ describe('multi-select', () => { it('renders selected values with a custom label', () => { render( undefined}> - `public.table_${value}`} /> + `Public.MixedCase_${value}`} /> ) - expect(screen.getByRole('combobox')).toHaveTextContent('public.table_101') + const badge = screen.getByText('Public.MixedCase_101').closest('[class*=rounded]') + expect(screen.getByRole('combobox')).toHaveTextContent('Public.MixedCase_101') + expect(badge).toHaveClass('normal-case', 'tracking-normal') }) it('opens the dropdown when the MultiSelectorTrigger is clicked', () => { @@ -78,6 +80,45 @@ describe('multi-select', () => { expect(screen.getByText('Apple')).toBeInTheDocument() // Apple should be visible in the dropdown }) + it('shows loading rows only inside the open dropdown', () => { + render( + undefined}> + + + + Apple + + + + ) + + expect(document.querySelector('.shimmering-loader')).not.toBeInTheDocument() + + fireEvent.click(screen.getByRole('combobox')) + + expect(document.querySelector('.shimmering-loader')).toBeInTheDocument() + expect(screen.queryByText('Apple')).not.toBeInTheDocument() + }) + + it('shows a custom loading error instead of an empty result', () => { + render( + undefined}> + + + + Apple + + + + ) + + fireEvent.click(screen.getByRole('combobox')) + + expect(screen.getByText('Unable to load fruits')).toBeInTheDocument() + expect(screen.queryByText('Apple')).not.toBeInTheDocument() + expect(screen.queryByText('No results found')).not.toBeInTheDocument() + }) + it('adds and removes value when toggling MultiSelectorItem', () => { render() diff --git a/packages/ui-patterns/src/multi-select/multi-select.tsx b/packages/ui-patterns/src/multi-select/multi-select.tsx index 9e5e63eca9c..766eae22f07 100644 --- a/packages/ui-patterns/src/multi-select/multi-select.tsx +++ b/packages/ui-patterns/src/multi-select/multi-select.tsx @@ -20,13 +20,15 @@ import { } from 'ui' import { SIZE_VARIANTS, SIZE_VARIANTS_DEFAULT } from 'ui/src/lib/constants' +import { SelectionListState } from '../SelectionListState' + interface MultiSelectContextProps { id: string values: string[] onValuesChange: (value: string[]) => void toggleValue: (values: string) => void open: boolean - setOpen: React.Dispatch> + setOpen: (open: boolean) => void inputValue: string setInputValue: React.Dispatch> activeIndex: number @@ -40,6 +42,7 @@ const MultiSelectContext = React.createContext(n const DROPDOWN_MAX_HEIGHT = 300 const DROPDOWN_GAP = 8 +const DROPDOWN_BORDER_HEIGHT = 2 const commandItemClass = cn( 'relative text-foreground-light text-left px-2 py-1.5 rounded-xs', @@ -72,6 +75,7 @@ type MultiSelectorProps = { mode?: MultiSelectorMode values: string[] onValuesChange: (value: string[]) => void + onOpenChange?: (open: boolean) => void disabled?: boolean } & React.ComponentPropsWithoutRef & VariantProps @@ -79,6 +83,7 @@ type MultiSelectorProps = { function MultiSelector({ values = [], onValuesChange, + onOpenChange, disabled, dir, size, @@ -88,13 +93,24 @@ function MultiSelector({ ...props }: MultiSelectorProps) { const ref = React.useRef(null) - const [open, setOpen] = React.useState(false) + const [open, setOpenState] = React.useState(false) const [inputValue, setInputValue] = React.useState('') const [activeIndex, setActiveIndex] = React.useState(-1) const [dropdownMaxHeight, setDropdownMaxHeight] = React.useState(DROPDOWN_MAX_HEIGHT) + const openRef = React.useRef(false) const generatedId = React.useId() const id = idProp ?? generatedId + const handleOpenChange = React.useCallback( + (nextOpen: boolean) => { + if (openRef.current === nextOpen) return + openRef.current = nextOpen + setOpenState(nextOpen) + onOpenChange?.(nextOpen) + }, + [onOpenChange] + ) + const toggleValue = React.useCallback( (toggledValue: string) => { if (values.includes(toggledValue)) { @@ -103,7 +119,7 @@ function MultiSelector({ onValuesChange([...values, toggledValue]) } }, - [values] + [onValuesChange, values] ) useEffect(() => { @@ -153,18 +169,18 @@ function MultiSelector({ } break case 'Escape': - activeIndex !== -1 ? setActiveIndex(-1) : setOpen(false) + activeIndex !== -1 ? setActiveIndex(-1) : handleOpenChange(false) if (ref.current) { const button = (ref.current as HTMLDivElement).querySelector('button[role="combobox"]') button && (button as HTMLButtonElement).focus() } break case 'Enter': - setOpen(true) + handleOpenChange(true) break } }, - [values, inputValue, activeIndex] + [values, inputValue, activeIndex, handleOpenChange] ) return ( @@ -175,7 +191,7 @@ function MultiSelector({ toggleValue, onValuesChange, open, - setOpen, + setOpen: handleOpenChange, inputValue, setInputValue, activeIndex, @@ -185,7 +201,7 @@ function MultiSelector({ dropdownMaxHeight, }} > - + = React.useCallback( (event) => { @@ -373,7 +390,7 @@ const MultiSelectorTrigger = React.forwardRef, React.ComponentPropsWithoutRef & { creatable?: boolean + emptyLabel?: string + error?: boolean + errorLabel?: string + loading?: boolean } ->(({ className, children, creatable = false, ...props }, ref) => { - const { open, inputValue, setInputValue, toggleValue, dropdownMaxHeight } = useMultiSelect() +>( + ( + { + className, + children, + creatable = false, + emptyLabel = 'No results found', + error = false, + errorLabel, + loading = false, + ...props + }, + ref + ) => { + const { open, inputValue, setInputValue, toggleValue, dropdownMaxHeight } = useMultiSelect() - const options = Children.toArray(children) - const availableOptions = options - .filter((x: any) => !!x.props.value) - .map((x: any) => x.props.value.toLowerCase()) - const isOptionExists = availableOptions.some((x: string) => x === inputValue.toLowerCase()) + const options = Children.toArray(children) + const availableOptions = options + .filter((x: any) => !!x.props.value) + .map((x: any) => x.props.value.toLowerCase()) + const isOptionExists = availableOptions.some((x: string) => x === inputValue.toLowerCase()) - return ( - e.stopPropagation()} - {...props} - > - {children} - {creatable && inputValue.length > 0 && !isOptionExists ? ( - { - open && toggleValue(inputValue) - setInputValue('') - }} - className={commandItemClass} - > - Create "{inputValue}" - - ) : creatable && options.length === 0 ? ( -
- Type to add a value -
- ) : ( - - No results found - - )} - - ) -}) + return ( + e.stopPropagation()} + {...props} + > + + {!loading && !error && (options.length > 0 || creatable) && ( + <> + {children} + {creatable && inputValue.length > 0 && !isOptionExists ? ( + { + open && toggleValue(inputValue) + setInputValue('') + }} + className={commandItemClass} + > + Create "{inputValue}" + + ) : creatable && options.length === 0 ? ( +
+ Type to add a value +
+ ) : ( + + {emptyLabel} + + )} + + )} + + ) + } +) MultiSelectorList.displayName = 'MultiSelectorList' MultiSelector.List = MultiSelectorList