From eeb06fcb1fb6d16eec01ce2d68f5a5291095f641 Mon Sep 17 00:00:00 2001 From: nguyenlongdang0412 Date: Sat, 26 Sep 2026 00:08:15 +0700 Subject: [PATCH] fix(input): make leading and trailing slots usable The two slots were part of the public API but had no test coverage, and three separate things were wrong with them. isLeading and isTrailing ignored the slots, so the padding compound variants never applied. The same icon measured 36px of padding through the leadingIcon prop and 12px through the slot, leaving the content overlapping the text by 22px. Both flags now account for the slots. The slot wrapper carries pointer-events-none so that a decorative icon stays click through and clicking it focuses the field. That also made interactive slot content impossible to click: a button in a slot never received a single click, while the cursor still changed on hover. Pointer events are now re-enabled only on a wrapper that actually holds a slot, and only while the field is neither disabled nor loading, so a control inside a disabled field stays inert. A slot also took precedence over the loading branch, so a field with a slot went disabled during loading with no spinner and no other indicator. Loading now wins, matching how Button swaps its leading icon for the spinner. Also corrected the loading prop documentation, which claimed it optionally disables interaction while the input is always disabled while loading. That text ships in the published type declarations. Closes #229 --- CHANGELOG.md | 1 + src/lib/components/Input/Input.svelte | 43 ++++++++---- src/lib/components/Input/Input.svelte.spec.ts | 66 +++++++++++++++++++ src/lib/components/Input/input.types.ts | 2 +- 4 files changed, 97 insertions(+), 15 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b2ec62e..62eff29 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- **Input** — `leadingSlot` and `trailingSlot` now reserve room in the field, so their content no longer sits on top of the text, and pointer events reach them, so a button placed in a slot is clickable. A decorative `leadingIcon` or `trailingIcon` stays click through, and slot content is inert while the field is disabled or loading. The loading spinner also takes precedence over a slot instead of being swallowed by it. ([#229](https://github.com/ndlabdev/sv5ui/issues/229)) - **Table** — the column resize handle is a focusable `separator` with `aria-valuenow`, arrow key resizing and a larger step while shift is held, plus `Home` and `End` for the bounds. It was mouse only before: no role, no tabindex and no key handler, so keyboard users could not resize a column at all. The drag also moves to the shared pointer hook, so touch and pen work rather than mouse alone. ([#226](https://github.com/ndlabdev/sv5ui/issues/226)) - **ThemeModeButton** — renders both mode icons and lets CSS pick the visible one, so server and client markup are identical. The wrong glyph no longer sticks after a reload in dark mode. The accessible name is now a mode-independent `Toggle theme`. ([#224](https://github.com/ndlabdev/sv5ui/issues/224)) - **Modal**, **Slideover**, **Drawer**, **Popover** — with `portal={false}`, nested floating layers (Select, DatePicker, DropdownMenu, Tooltip, ...) no longer hide behind the container when an ancestor has a `z-index` above 50. ([#217](https://github.com/ndlabdev/sv5ui/issues/217)) diff --git a/src/lib/components/Input/Input.svelte b/src/lib/components/Input/Input.svelte index 41cb999..d05fb0c 100644 --- a/src/lib/components/Input/Input.svelte +++ b/src/lib/components/Input/Input.svelte @@ -97,8 +97,15 @@ const loadingLeading = $derived(loading && !trailing) const loadingTrailing = $derived(loading && trailing) - const isLeading = $derived((!!icon && !trailing) || !!leadingIcon || !!avatar || loadingLeading) - const isTrailing = $derived((!!icon && trailing) || !!trailingIcon || loadingTrailing) + const isInert = $derived(disabled || loading) + const leadingInteractive = $derived(!!leadingSlot && !isInert) + const trailingInteractive = $derived(!!trailingSlot && !isInert) + const isLeading = $derived( + !!leadingSlot || (!!icon && !trailing) || !!leadingIcon || !!avatar || loadingLeading + ) + const isTrailing = $derived( + !!trailingSlot || (!!icon && trailing) || !!trailingIcon || loadingTrailing + ) const leadingIconName = $derived(leadingIcon || (!!icon && !trailing ? icon : undefined)) const trailingIconName = $derived(trailingIcon || (!!icon && trailing ? icon : undefined)) @@ -128,7 +135,9 @@ base: variantSlots.base({ class: [config.slots.base, fieldGroupClass?.base, ui?.base] }), - leading: variantSlots.leading({ class: [config.slots.leading, ui?.leading] }), + leading: variantSlots.leading({ + class: [config.slots.leading, leadingInteractive && 'pointer-events-auto', ui?.leading] + }), leadingIcon: variantSlots.leadingIcon({ class: [config.slots.leadingIcon, ui?.leadingIcon] }), @@ -136,7 +145,13 @@ class: [config.slots.leadingAvatar, ui?.leadingAvatar] }), leadingAvatarSize: variantSlots.leadingAvatarSize() as AvatarSize, - trailing: variantSlots.trailing({ class: [config.slots.trailing, ui?.trailing] }), + trailing: variantSlots.trailing({ + class: [ + config.slots.trailing, + trailingInteractive && 'pointer-events-auto', + ui?.trailing + ] + }), trailingIcon: variantSlots.trailingIcon({ class: [config.slots.trailingIcon, ui?.trailingIcon] }) @@ -144,16 +159,16 @@
- {#if leadingSlot} - - {@render leadingSlot()} - - {:else if loadingLeading} + {#if loadingLeading} + {:else if leadingSlot} + + {@render leadingSlot()} + {:else if avatar} @@ -181,16 +196,16 @@ onfocus={handleFocus} /> - {#if trailingSlot} - - {@render trailingSlot()} - - {:else if loadingTrailing} + {#if loadingTrailing} + {:else if trailingSlot} + + {@render trailingSlot()} + {:else if trailingIconName} diff --git a/src/lib/components/Input/Input.svelte.spec.ts b/src/lib/components/Input/Input.svelte.spec.ts index a820f70..120d574 100644 --- a/src/lib/components/Input/Input.svelte.spec.ts +++ b/src/lib/components/Input/Input.svelte.spec.ts @@ -1,8 +1,12 @@ +import '../../../routes/layout.css' import { page } from 'vitest/browser' import { describe, expect, it, vi } from 'vitest' import { render } from 'vitest-browser-svelte' +import { createRawSnippet } from 'svelte' import Input from './Input.svelte' +const snippet = (html: string) => createRawSnippet(() => ({ render: () => html, setup: () => {} })) + const AVATAR_SRC = 'data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR42mNk+M9QDwADhgGAWjR9awAAAABJRU5ErkJggg==' @@ -402,6 +406,68 @@ describe('Input', () => { // ==================== ACCESSIBILITY ==================== + // ==================== LEADING & TRAILING SLOTS ==================== + + describe('leading and trailing slots', () => { + const button = () => snippet('') + + const wrapperOf = (container: Element, side: 'first' | 'last') => { + const spans = Array.from(container.querySelectorAll('div > span')) + return side === 'first' ? spans[0] : spans[spans.length - 1] + } + + it('should reserve room for a leading slot, like the leadingIcon prop does', () => { + const { container } = render(Input, { leadingSlot: button() }) + const input = container.querySelector('input')! + + expect(input.className).toMatch(/\bps-\d/) + }) + + it('should reserve room for a trailing slot', () => { + const { container } = render(Input, { trailingSlot: button() }) + const input = container.querySelector('input')! + + expect(input.className).toMatch(/\bpe-\d/) + }) + + it('should let pointer events reach interactive slot content', () => { + const { container } = render(Input, { trailingSlot: button() }) + + expect(getComputedStyle(wrapperOf(container, 'last')).pointerEvents).toBe('auto') + }) + + it('should keep a decorative icon click through so it focuses the input', () => { + const { container } = render(Input, { trailingIcon: 'lucide:check' }) + + expect(getComputedStyle(wrapperOf(container, 'last')).pointerEvents).toBe('none') + }) + + it.each([ + ['disabled', { disabled: true }], + ['loading', { loading: true }] + ])('should block slot interaction while the field is %s', (_label, props) => { + const { container } = render(Input, { ...props, trailingSlot: button() }) + + expect(getComputedStyle(wrapperOf(container, 'last')).pointerEvents).toBe('none') + }) + + it('should still show the loading spinner when a slot is present', async () => { + const { container } = render(Input, { loading: true, leadingSlot: button() }) + + await vi.waitFor(() => { + expect(container.querySelector('.animate-spin')).not.toBeNull() + }) + expect(container.querySelector('#slot-btn')).toBeNull() + }) + + it('should render slot content when not loading', () => { + const { container } = render(Input, { leadingSlot: button() }) + + expect(container.querySelector('#slot-btn')).not.toBeNull() + expect(container.querySelector('.animate-spin')).toBeNull() + }) + }) + describe('accessibility', () => { it('should support aria-label', () => { render(Input, { 'aria-label': 'Search input' }) diff --git a/src/lib/components/Input/input.types.ts b/src/lib/components/Input/input.types.ts index d99af69..922c0ba 100644 --- a/src/lib/components/Input/input.types.ts +++ b/src/lib/components/Input/input.types.ts @@ -66,7 +66,7 @@ export type InputProps = Omit< highlight?: boolean /** - * Renders a loading spinner and optionally disables interaction. + * Renders a loading spinner. The input is disabled while loading. * @default false */ loading?: boolean