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.
This commit is contained in:
@@ -6,11 +6,13 @@
|
|||||||
<script lang="ts">
|
<script lang="ts">
|
||||||
import type { TypographyControl } from '$shared/lib';
|
import type { TypographyControl } from '$shared/lib';
|
||||||
import { cn } 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 { Button } from '$shared/ui/Button';
|
||||||
import MinusIcon from '@lucide/svelte/icons/minus';
|
import MinusIcon from '@lucide/svelte/icons/minus';
|
||||||
import PlusIcon from '@lucide/svelte/icons/plus';
|
import PlusIcon from '@lucide/svelte/icons/plus';
|
||||||
import { Popover } from 'bits-ui';
|
|
||||||
import TechText from '../TechText/TechText.svelte';
|
import TechText from '../TechText/TechText.svelte';
|
||||||
|
|
||||||
interface Props {
|
interface Props {
|
||||||
@@ -114,59 +116,55 @@ const displayLabel = $derived(label ?? controlLabel ?? '');
|
|||||||
|
|
||||||
<!-- Trigger -->
|
<!-- Trigger -->
|
||||||
<div class="relative mx-1">
|
<div class="relative mx-1">
|
||||||
<Popover.Root bind:open>
|
<Popover bind:open side="top" align="center">
|
||||||
<Popover.Trigger>
|
{#snippet trigger(props)}
|
||||||
{#snippet child({ props })}
|
<button
|
||||||
<button
|
{...props}
|
||||||
{...props}
|
class={cn(
|
||||||
class={cn(
|
'flex flex-col flex-center w-14 py-1',
|
||||||
'flex flex-col flex-center w-14 py-1',
|
'select-none rounded-none transition-all duration-fast',
|
||||||
'select-none rounded-none transition-all duration-fast',
|
'border border-transparent',
|
||||||
'border border-transparent',
|
'focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-brand/30',
|
||||||
'focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-brand/30',
|
open
|
||||||
open
|
? 'surface-card-elevated'
|
||||||
? 'surface-card-elevated'
|
: 'hover:bg-paper/50 dark:hover:bg-dark-card/50',
|
||||||
: 'hover:bg-paper/50 dark:hover:bg-dark-card/50',
|
)}
|
||||||
)}
|
aria-label={controlLabel ? `${controlLabel}: ${formattedValue()}` : undefined}
|
||||||
aria-label={controlLabel ? `${controlLabel}: ${formattedValue()}` : undefined}
|
>
|
||||||
>
|
<!-- Label row -->
|
||||||
<!-- Label row -->
|
{#if displayLabel}
|
||||||
{#if displayLabel}
|
<span
|
||||||
<span
|
class="
|
||||||
class="
|
text-3xs text-label-mono
|
||||||
text-3xs text-label-mono
|
text-neutral-900 dark:text-neutral-100
|
||||||
text-neutral-900 dark:text-neutral-100
|
mb-0.5 leading-none
|
||||||
mb-0.5 leading-none
|
"
|
||||||
"
|
>
|
||||||
>
|
{displayLabel}
|
||||||
{displayLabel}
|
</span>
|
||||||
</span>
|
{/if}
|
||||||
{/if}
|
|
||||||
|
|
||||||
<!-- Value row -->
|
<!-- Value row -->
|
||||||
<TechText variant="muted" size="md">
|
<TechText variant="muted" size="md">
|
||||||
{formattedValue()}
|
{formattedValue()}
|
||||||
</TechText>
|
</TechText>
|
||||||
</button>
|
</button>
|
||||||
{/snippet}
|
{/snippet}
|
||||||
</Popover.Trigger>
|
|
||||||
|
|
||||||
<!-- Vertical slider popover -->
|
<!-- Vertical slider popover -->
|
||||||
<Popover.Content
|
{#snippet children()}
|
||||||
class="w-auto py-4 px-3 h-64 flex-center rounded-none surface-card-elevated"
|
<div class="w-auto py-4 px-3 h-64 flex-center rounded-none surface-card-elevated">
|
||||||
align="center"
|
<Slider
|
||||||
side="top"
|
class="h-full"
|
||||||
>
|
bind:value={control.value}
|
||||||
<Slider
|
min={control.min}
|
||||||
class="h-full"
|
max={control.max}
|
||||||
bind:value={control.value}
|
step={control.step}
|
||||||
min={control.min}
|
orientation="vertical"
|
||||||
max={control.max}
|
/>
|
||||||
step={control.step}
|
</div>
|
||||||
orientation="vertical"
|
{/snippet}
|
||||||
/>
|
</Popover>
|
||||||
</Popover.Content>
|
|
||||||
</Popover.Root>
|
|
||||||
</div>
|
</div>
|
||||||
|
|
||||||
<!-- Increase button -->
|
<!-- Increase button -->
|
||||||
|
|||||||
@@ -4,6 +4,7 @@ import {
|
|||||||
render,
|
render,
|
||||||
screen,
|
screen,
|
||||||
waitFor,
|
waitFor,
|
||||||
|
within,
|
||||||
} from '@testing-library/svelte';
|
} from '@testing-library/svelte';
|
||||||
import ComboControl from './ComboControl.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('ComboControl', () => {
|
||||||
describe('Rendering', () => {
|
describe('Rendering', () => {
|
||||||
it('renders decrease and increase buttons', () => {
|
it('renders decrease and increase buttons', () => {
|
||||||
@@ -26,17 +37,17 @@ describe('ComboControl', () => {
|
|||||||
|
|
||||||
it('renders the current integer value', () => {
|
it('renders the current integer value', () => {
|
||||||
render(ComboControl, { control: makeControl(42) });
|
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', () => {
|
it('formats decimal value to 1 decimal place when step >= 0.1', () => {
|
||||||
render(ComboControl, { control: makeControl(1.5, { 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', () => {
|
it('formats decimal value to 2 decimal places when step < 0.1', () => {
|
||||||
render(ComboControl, { control: makeControl(1.55, { step: 0.01 }) });
|
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', () => {
|
it('renders label when label prop is provided', () => {
|
||||||
@@ -106,16 +117,32 @@ describe('ComboControl', () => {
|
|||||||
const control = makeControl(50);
|
const control = makeControl(50);
|
||||||
render(ComboControl, { control });
|
render(ComboControl, { control });
|
||||||
await fireEvent.click(screen.getByLabelText('Increase'));
|
await fireEvent.click(screen.getByLabelText('Increase'));
|
||||||
await waitFor(() => expect(screen.getByText('51')).toBeInTheDocument());
|
await waitFor(() => expect(within(getTrigger()).getByText('51')).toBeInTheDocument());
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
describe('Popover', () => {
|
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' });
|
render(ComboControl, { control: makeControl(50), controlLabel: 'Size control' });
|
||||||
expect(screen.queryByRole('slider')).not.toBeInTheDocument();
|
|
||||||
await fireEvent.click(screen.getByText('Size control'));
|
const trigger = getTrigger();
|
||||||
await waitFor(() => expect(screen.getByRole('slider')).toBeInTheDocument());
|
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');
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user