Skip to content

fix: Message Grouping card form improvements #1420

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.

Sign up for GitHub

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

Merged
acco merged 3 commits into from
May 13, 2025
Merged

fix: Message Grouping card form improvements #1420

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
968054e
fix: use expandable cards for group columns form
davoclavo May 10, 2025
f857be3
feat: add tooltip for group message cards for existing sinks
davoclavo May 10, 2025
6037005
fix: misc fixes to sink forms, and expandable cards
davoclavo May 13, 2025
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
35 changes: 35 additions & 0 deletions assets/svelte/components/ui/card/expandable-card-header.svelte
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
<script lang="ts">
import { CardHeader, CardTitle } from ".";
import { ChevronDown } from "lucide-svelte";
import { Button as ButtonPrimitive } from "bits-ui";

export let expanded: boolean = false;
export let disabled: boolean = false;
export let builders: $$Props["builders"] = [];

Comment on lines +8 to +9
Copy link

Copilot AI May 12, 2025

Choose a reason for hiding this comment

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

[nitpick] Clarify and document the expected structure and type for the 'builders' prop to ensure future maintainability and readability.

Suggested change
export let builders: $$Props["builders"] = [];
/**
* An array of builder objects used to customize the behavior or appearance of the button.
* Each builder object should have the following structure:
* {
* id: string; // A unique identifier for the builder
* label: string; // A label describing the builder
* action: () => void; // A function to execute when the builder is triggered
* }
*/
export let builders: Builder[] = [];
// Define the Builder type
type Builder = {
id: string;
label: string;
action: () => void;
};

Copilot uses AI. Check for mistakes.
function handleClick() {
if (disabled) return;
expanded = !expanded;
}
</script>

<CardHeader class="flex flex-row items-center justify-between">
<ButtonPrimitive.Root
class="flex flex-row items-center justify-between w-full cursor-pointer"
on:click={handleClick}
type="button"
{builders}
{disabled}
>
<CardTitle class="flex items-center gap-2">
<slot />
</CardTitle>
<div
class="transition-transform duration-200"
class:rotate-180={expanded}
class:text-gray-300={disabled}
>
<ChevronDown class="h-4 w-4" />
</div>
</ButtonPrimitive.Root>
</CardHeader>
52 changes: 26 additions & 26 deletions assets/svelte/components/ui/card/expandable-card.svelte
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,40 +1,40 @@
<script lang="ts">
import { Card, CardContent, CardHeader, CardTitle } from ".";
import { ChevronDown } from "lucide-svelte";
import { Card, CardContent } from ".";
import * as Tooltip from "$lib/components/ui/tooltip";
import { Info } from "lucide-svelte";
import ExpandableCardHeader from "./expandable-card-header.svelte";

export let expanded: boolean = false;
export let disabled: boolean = false;

function handleClick() {
if (disabled) return;
expanded = !expanded;
}
export let titleTooltip: string | null = null;
</script>

<Card>
<CardHeader class="flex flex-row items-center justify-between">
<button
type="button"
class="flex flex-row items-center justify-between w-full"
on:click={handleClick}
{disabled}
{#if titleTooltip}
<Tooltip.Root
openDelay={10}
Copy link

Copilot AI May 12, 2025

Choose a reason for hiding this comment

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

[nitpick] Consider if the openDelay of 10 ms is sufficient for accessibility; a slightly longer delay might reduce accidental tooltip activations and improve usability.

Suggested change
openDelay={10}
openDelay={300}

Copilot uses AI. Check for mistakes.
class="flex flex-row items-center justify-between"
>
<CardTitle class="flex items-center gap-2">
<slot name="title" />
</CardTitle>
<div
class="transition-transform duration-200"
class:rotate-180={expanded}
class:text-gray-300={disabled}
>
<ChevronDown class="h-4 w-4" />
</div>
</button>
</CardHeader>
<Tooltip.Trigger asChild let:builder>
<ExpandableCardHeader builders={[builder]} bind:expanded {disabled}>
<slot name="title" />
</ExpandableCardHeader>
</Tooltip.Trigger>
<Tooltip.Content class="max-w-xs space-y-2">
<p class="text-xs text-gray-500">
{titleTooltip}
</p>
</Tooltip.Content>
</Tooltip.Root>
{:else}
<ExpandableCardHeader bind:expanded {disabled}>
<slot name="title" />
</ExpandableCardHeader>
{/if}

<CardContent>
<div class="space-y-2">
{#if disabled || !expanded}
{#if !expanded}
<slot name="summary" />
{:else}
<slot name="content" />
Expand Down
53 changes: 32 additions & 21 deletions assets/svelte/consumers/GroupColumnsForm.svelte
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
let defaultGroupColumns = selectedTable?.default_group_columns || [];
let groupColumnError: string | null = null;
let useCustomGrouping = false;
let isExpanded = false;

$: groupColumnError = errors.sequence_filter?.group_column_attnums?.[0];
$: defaultGroupColumns = selectedTable?.default_group_columns || [];
Expand Down Expand Up @@ -60,18 +59,22 @@
.join(", ")}`
: selectedTable?.is_event_table
? "Using source_database_id, source_table_oid, and record_pk for grouping, which is the default"
: "Using primary keys for grouping, which is the default";
: defaultGroupColumns.length === 0
? "No primary keys available. Custom grouping is required."
: "Using primary keys for grouping, which is the default";
</script>

<!-- Edit Mode Card -->
{#if isEditMode}
<Card>
<CardHeader>
<div class="flex items-center justify-between">
<CardTitle>Message grouping</CardTitle>
</div>
</CardHeader>
<CardContent>
<ExpandableCard
disabled={true}
titleTooltip="Message grouping can’t be changed for an active sink."
>
<svelte:fragment slot="title">
<CardTitle>Message grouping</CardTitle>
</svelte:fragment>

<svelte:fragment slot="summary">
{#if !useCustomGrouping && !selectedTable.is_event_table}
<p class="text-sm text-muted-foreground">
Using primary keys for grouping.
Expand All @@ -83,16 +86,26 @@
readonly={true}
/>
{/if}
</CardContent>
</Card>
</svelte:fragment>
</ExpandableCard>

<!-- Create Mode - No PKs Available -->
{:else if selectedTable && defaultGroupColumns.length === 0}
<Card>
<CardHeader>
<ExpandableCard
expanded={!isEditMode}
disabled={defaultGroupColumns.length === 0}
>
<svelte:fragment slot="title">
<CardTitle>Message grouping</CardTitle>
</CardHeader>
<CardContent>
</svelte:fragment>

<svelte:fragment slot="summary">
<p class="text-sm text-muted-foreground">
{summaryText}
</p>
</svelte:fragment>

<svelte:fragment slot="content">
<p class="text-sm text-info mb-4">
No primary keys available. Custom grouping is required.
</p>
Expand All @@ -104,16 +117,14 @@
{#if groupColumnError}
<p class="text-destructive text-sm mt-2">{groupColumnError}</p>
{/if}
</CardContent>
</Card>
</svelte:fragment>
</ExpandableCard>

<!-- Create Mode - Normal -->
{:else}
<ExpandableCard disabled={!selectedTable} expanded={!isEditMode}>
<ExpandableCard disabled={!selectedTable}>
<svelte:fragment slot="title">
<div class="flex items-center justify-between">
<CardTitle>Message grouping</CardTitle>
</div>
<CardTitle>Message grouping</CardTitle>
</svelte:fragment>

<svelte:fragment slot="summary">
Expand Down
16 changes: 13 additions & 3 deletions assets/svelte/consumers/SinkConsumerForm.svelte
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -354,15 +354,19 @@
});
}

let backfillSectionExpanded = false;
let showMessageTypeExampleModal = false;
let selectedExampleType: "change" | "record" = "change";

let transformSectionEnabled = false;
let transformSectionExpanded = false;
let backfillSectionEnabled = false;
let backfillSectionExpanded = false;
$: {
transformSectionEnabled = selectedTable && consumer.type !== "redis_stream";
transformSectionExpanded = transformSectionEnabled && !isEditMode;

backfillSectionEnabled = selectedTable && !isEditMode;
backfillSectionExpanded = backfillSectionEnabled && !isEditMode;
}

let transformRefreshState: "idle" | "refreshing" | "done" = "idle";
Expand Down Expand Up @@ -521,7 +525,10 @@
</CardContent>
</Card>

<ExpandableCard disabled={!transformSectionEnabled} expanded={!isEditMode}>
<ExpandableCard
disabled={!transformSectionEnabled}
expanded={transformSectionExpanded}
>
<svelte:fragment slot="title">
Transforms
<Tooltip.Root openDelay={200}>
Expand Down Expand Up @@ -582,7 +589,10 @@
</ExpandableCard>

{#if !isEditMode}
<ExpandableCard disabled={!backfillSectionEnabled} expanded={!isEditMode}>
<ExpandableCard
disabled={!backfillSectionEnabled}
expanded={!isEditMode && backfillSectionEnabled}
>
<svelte:fragment slot="title">
Initial backfill

Expand Down
Loading