From 7798c4bbdfa94c5457d63b5eece23082943ddcfc Mon Sep 17 00:00:00 2001
From: Ilia Mashkov <hikkikomori7@gmail.com>
Date: Tue, 2 Jun 2026 16:21:32 +0300
Subject: [PATCH] refactor(combo-control): use native Popover instead of
 bits-ui

The native Popover always renders its content (the vertical slider), so the
slider's value label is in the DOM even when closed, and opening is driven by
the browser's declarative popovertarget invoker (not simulated by jsdom on
click). Update the tests to scope value assertions to the trigger and drive
open via showPopover(), matching Popover.svelte.test.ts.
---
 .../ui/ComboControl/ComboControl.svelte       | 102 +++++++++---------
 .../ComboControl/ComboControl.svelte.test.ts  |  43 ++++++--
 2 files changed, 85 insertions(+), 60 deletions(-)

diff --git a/src/shared/ui/ComboControl/ComboControl.svelte b/src/shared/ui/ComboControl/ComboControl.svelte
index 778274e..1371c6d 100644
--- a/src/shared/ui/ComboControl/ComboControl.svelte
+++ b/src/shared/ui/ComboControl/ComboControl.svelte
@@ -6,11 +6,13 @@
 <script lang="ts">
 import type { TypographyControl } from '$shared/lib';
 import { cn } from '$shared/lib';
-import { Slider } from '$shared/ui';
+import {
+    Popover,
+    Slider,
+} from '$shared/ui';
 import { Button } from '$shared/ui/Button';
 import MinusIcon from '@lucide/svelte/icons/minus';
 import PlusIcon from '@lucide/svelte/icons/plus';
-import { Popover } from 'bits-ui';
 import TechText from '../TechText/TechText.svelte';
 
 interface Props {
@@ -114,59 +116,55 @@ const displayLabel = $derived(label ?? controlLabel ?? '');
 
         <!-- Trigger -->
         <div class="relative mx-1">
-            <Popover.Root bind:open>
-                <Popover.Trigger>
-                    {#snippet child({ props })}
-                        <button
-                            {...props}
-                            class={cn(
-                                'flex flex-col flex-center w-14 py-1',
-                                'select-none rounded-none transition-all duration-fast',
-                                'border border-transparent',
-                                'focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-brand/30',
-                                open
-                                    ? 'surface-card-elevated'
-                                    : 'hover:bg-paper/50 dark:hover:bg-dark-card/50',
-                            )}
-                            aria-label={controlLabel ? `${controlLabel}: ${formattedValue()}` : undefined}
-                        >
-                            <!-- Label row -->
-                            {#if displayLabel}
-                                <span
-                                    class="
-                                        text-3xs text-label-mono
-                                        text-neutral-900 dark:text-neutral-100
-                                        mb-0.5 leading-none
-                                    "
-                                >
-                                    {displayLabel}
-                                </span>
-                            {/if}
+            <Popover bind:open side="top" align="center">
+                {#snippet trigger(props)}
+                    <button
+                        {...props}
+                        class={cn(
+                            'flex flex-col flex-center w-14 py-1',
+                            'select-none rounded-none transition-all duration-fast',
+                            'border border-transparent',
+                            'focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-brand/30',
+                            open
+                                ? 'surface-card-elevated'
+                                : 'hover:bg-paper/50 dark:hover:bg-dark-card/50',
+                        )}
+                        aria-label={controlLabel ? `${controlLabel}: ${formattedValue()}` : undefined}
+                    >
+                        <!-- Label row -->
+                        {#if displayLabel}
+                            <span
+                                class="
+                                    text-3xs text-label-mono
+                                    text-neutral-900 dark:text-neutral-100
+                                    mb-0.5 leading-none
+                                "
+                            >
+                                {displayLabel}
+                            </span>
+                        {/if}
 
-                            <!-- Value row -->
-                            <TechText variant="muted" size="md">
-                                {formattedValue()}
-                            </TechText>
-                        </button>
-                    {/snippet}
-                </Popover.Trigger>
+                        <!-- Value row -->
+                        <TechText variant="muted" size="md">
+                            {formattedValue()}
+                        </TechText>
+                    </button>
+                {/snippet}
 
                 <!-- Vertical slider popover -->
-                <Popover.Content
-                    class="w-auto py-4 px-3 h-64 flex-center rounded-none surface-card-elevated"
-                    align="center"
-                    side="top"
-                >
-                    <Slider
-                        class="h-full"
-                        bind:value={control.value}
-                        min={control.min}
-                        max={control.max}
-                        step={control.step}
-                        orientation="vertical"
-                    />
-                </Popover.Content>
-            </Popover.Root>
+                {#snippet children()}
+                    <div class="w-auto py-4 px-3 h-64 flex-center rounded-none surface-card-elevated">
+                        <Slider
+                            class="h-full"
+                            bind:value={control.value}
+                            min={control.min}
+                            max={control.max}
+                            step={control.step}
+                            orientation="vertical"
+                        />
+                    </div>
+                {/snippet}
+            </Popover>
         </div>
 
         <!-- Increase button -->
diff --git a/src/shared/ui/ComboControl/ComboControl.svelte.test.ts b/src/shared/ui/ComboControl/ComboControl.svelte.test.ts
index 8eade6a..61e2e35 100644
--- a/src/shared/ui/ComboControl/ComboControl.svelte.test.ts
+++ b/src/shared/ui/ComboControl/ComboControl.svelte.test.ts
@@ -4,6 +4,7 @@ import {
     render,
     screen,
     waitFor,
+    within,
 } from '@testing-library/svelte';
 import ComboControl from './ComboControl.svelte';
 
@@ -16,6 +17,16 @@ function makeControl(value: number, opts: { min?: number; max?: number; step?: n
     });
 }
 
+/**
+ * The trigger is the button wired to the popover (has popovertarget). The native
+ * Popover always renders its content (the vertical slider, which also displays the
+ * value) in the DOM, so value assertions must be scoped to the trigger to avoid
+ * matching the slider's own value label.
+ */
+function getTrigger(): HTMLElement {
+    return document.querySelector('button[popovertarget]') as HTMLElement;
+}
+
 describe('ComboControl', () => {
     describe('Rendering', () => {
         it('renders decrease and increase buttons', () => {
@@ -26,17 +37,17 @@ describe('ComboControl', () => {
 
         it('renders the current integer value', () => {
             render(ComboControl, { control: makeControl(42) });
-            expect(screen.getByText('42')).toBeInTheDocument();
+            expect(within(getTrigger()).getByText('42')).toBeInTheDocument();
         });
 
         it('formats decimal value to 1 decimal place when step >= 0.1', () => {
             render(ComboControl, { control: makeControl(1.5, { step: 0.1 }) });
-            expect(screen.getByText('1.5')).toBeInTheDocument();
+            expect(within(getTrigger()).getByText('1.5')).toBeInTheDocument();
         });
 
         it('formats decimal value to 2 decimal places when step < 0.1', () => {
             render(ComboControl, { control: makeControl(1.55, { step: 0.01 }) });
-            expect(screen.getByText('1.55')).toBeInTheDocument();
+            expect(within(getTrigger()).getByText('1.55')).toBeInTheDocument();
         });
 
         it('renders label when label prop is provided', () => {
@@ -106,16 +117,32 @@ describe('ComboControl', () => {
             const control = makeControl(50);
             render(ComboControl, { control });
             await fireEvent.click(screen.getByLabelText('Increase'));
-            await waitFor(() => expect(screen.getByText('51')).toBeInTheDocument());
+            await waitFor(() => expect(within(getTrigger()).getByText('51')).toBeInTheDocument());
         });
     });
 
     describe('Popover', () => {
-        it('opens popover with vertical slider on trigger click', async () => {
+        /**
+         * The native Popover always renders its content; opening is driven by the
+         * browser's declarative popovertarget invoker, which jsdom does not simulate
+         * on click (mirrors Popover.svelte.test.ts). So assert the wired-but-closed
+         * state, then drive the open through the API the browser would call.
+         */
+        it('exposes a popover trigger with the vertical slider as its content', async () => {
             render(ComboControl, { control: makeControl(50), controlLabel: 'Size control' });
-            expect(screen.queryByRole('slider')).not.toBeInTheDocument();
-            await fireEvent.click(screen.getByText('Size control'));
-            await waitFor(() => expect(screen.getByRole('slider')).toBeInTheDocument());
+
+            const trigger = getTrigger();
+            expect(trigger).toHaveAttribute('aria-expanded', 'false');
+
+            const content = document.getElementById(trigger.getAttribute('popovertarget')!) as HTMLElement;
+            expect(content).toHaveAttribute('data-state', 'closed');
+            // The vertical slider lives inside the popover content. While closed the
+            // content is visibility:hidden, so query including hidden elements.
+            expect(within(content).getByRole('slider', { hidden: true })).toBeInTheDocument();
+
+            content.showPopover();
+            await waitFor(() => expect(content).toHaveAttribute('data-state', 'open'));
+            expect(trigger).toHaveAttribute('aria-expanded', 'true');
         });
     });