diff --git a/packages/craftcms-ui/src/components/combobox/combobox.styles.ts b/packages/craftcms-ui/src/components/combobox/combobox.styles.ts index b619c53bf97..130002b38a6 100644 --- a/packages/craftcms-ui/src/components/combobox/combobox.styles.ts +++ b/packages/craftcms-ui/src/components/combobox/combobox.styles.ts @@ -65,7 +65,6 @@ export default css` :host([multiple-choice]) .input-group__input { ${baseFormControlStyles} - box-sizing: border-box; display: flex; flex-wrap: wrap; align-items: center; diff --git a/packages/craftcms-ui/src/components/field/field.browser.test.ts b/packages/craftcms-ui/src/components/field/field.browser.test.ts index c51e04d05bd..21ea1f75461 100644 --- a/packages/craftcms-ui/src/components/field/field.browser.test.ts +++ b/packages/craftcms-ui/src/components/field/field.browser.test.ts @@ -1,5 +1,6 @@ -import {beforeEach, expect, it, vi} from 'vite-plus/test'; +import {beforeEach, describe, expect, it, vi} from 'vite-plus/test'; import {computeAccessibleName} from 'dom-accessibility-api'; +import type CraftField from './field.js'; import './field.js'; import '../input/input.js'; import '../select/select.js'; @@ -20,3 +21,47 @@ it('labels nested native controls', async () => { 'Operator' ); }); + +describe('spacing', () => { + async function renderField(attrs: string): Promise { + document.body.innerHTML = ``; + const field = document.querySelector('craft-field')!; + await field.updateComplete; + return field; + } + + /** Distance from the top of the field to the top of the input group. */ + function inputOffset(field: CraftField): number { + const inputGroup = field.shadowRoot!.querySelector( + '.form-field__group-two' + )!; + return ( + inputGroup.getBoundingClientRect().top - field.getBoundingClientRect().top + ); + } + + it('leaves no space above the input without a label', async () => { + expect(inputOffset(await renderField(''))).toBe(0); + }); + + it('leaves no space above the input for a visually hidden label', async () => { + const field = await renderField('label="Title" label-sr-only'); + + expect(inputOffset(field)).toBe(0); + expect(computeAccessibleName(field.querySelector('input')!)).toBe('Title'); + }); + + it('spaces instructions from the input when the label is visually hidden', async () => { + const field = await renderField( + 'label="Title" label-sr-only help-text="Some instructions"' + ); + const helpText = field.shadowRoot!.querySelector('.form-field__help-text')!; + + expect( + helpText.getBoundingClientRect().top - field.getBoundingClientRect().top + ).toBe(0); + expect(inputOffset(field)).toBeGreaterThan( + helpText.getBoundingClientRect().height + ); + }); +}); diff --git a/packages/craftcms-ui/src/components/field/field.styles.ts b/packages/craftcms-ui/src/components/field/field.styles.ts index e01e2dd5284..68b8022dcc7 100644 --- a/packages/craftcms-ui/src/components/field/field.styles.ts +++ b/packages/craftcms-ui/src/components/field/field.styles.ts @@ -54,7 +54,6 @@ export default css` gap: 5px; align-items: center; font-weight: bold; - margin-block-end: var(--c-spacing-xs, 0.25rem); } /* Pushes slotted label extras to the far end of the heading row. */ diff --git a/packages/craftcms-ui/src/components/field/field.test.ts b/packages/craftcms-ui/src/components/field/field.test.ts index d92de8422c8..1eccf2951d6 100644 --- a/packages/craftcms-ui/src/components/field/field.test.ts +++ b/packages/craftcms-ui/src/components/field/field.test.ts @@ -471,6 +471,131 @@ describe('craft-field instructions position', () => { }); }); +describe('craft-field empty label and instructions', () => { + function heading(element: CraftField): Element | null { + return element.shadowRoot!.querySelector('.form-field__label'); + } + + function helpText(element: CraftField): Element | null { + return element.shadowRoot!.querySelector('.form-field__help-text'); + } + + it('renders no heading without a label', async () => { + const element = await createField(); + expect(heading(element)).toBeNull(); + }); + + it('renders no instructions without help text', async () => { + const element = await createField({label: 'My field'}); + expect(helpText(element)).toBeNull(); + }); + + it('renders a slotted label and instructions', async () => { + const element = await createField( + {}, + '
Custom instructions
' + ); + + expect( + heading(element)!.querySelector('slot[name="label"]') + ).not.toBeNull(); + expect(helpText(element)).not.toBeNull(); + }); + + it('renders the heading and instructions once they are set', async () => { + const element = await createField(); + + element.label = 'My field'; + element.helpText = 'Some instructions'; + await element.updateComplete; + + expect(heading(element)).not.toBeNull(); + expect(helpText(element)).not.toBeNull(); + }); + + it('drops the heading and instructions once they are cleared', async () => { + const element = await createField({ + label: 'My field', + 'help-text': 'Some instructions', + }); + + element.label = ''; + element.helpText = ''; + await element.updateComplete; + + expect(heading(element)).toBeNull(); + expect(helpText(element)).toBeNull(); + }); + + it('keeps the heading for other heading content without a label', async () => { + const element = await createField( + {}, + '' + ); + + expect(heading(element)).not.toBeNull(); + expect(heading(element)!.querySelector('slot[name="label"]')).toBeNull(); + expect( + heading(element)!.querySelector('slot[name="actions"]') + ).not.toBeNull(); + }); + + it('keeps the heading for the read-only badge without a label', async () => { + const element = await createField({readonly: ''}); + expect(heading(element)!.querySelector('.read-only-badge')).not.toBeNull(); + }); + + it('renders the heading once heading content is slotted later', async () => { + const element = await createField(); + + const action = document.createElement('button'); + action.slot = 'actions'; + element.append(action); + await new Promise((resolve) => setTimeout(resolve)); + await element.updateComplete; + + expect(heading(element)).not.toBeNull(); + }); +}); + +describe('craft-field label-sr-only', () => { + it('visually hides the whole heading when the label is all it has', async () => { + const element = await createField({ + label: 'My field', + 'label-sr-only': '', + }); + + const heading = element.shadowRoot!.querySelector('.form-field__label')!; + expect(heading.classList.contains('cp-visually-hidden')).toBe(true); + expect(heading.querySelector('slot[name="label"]')).not.toBeNull(); + }); + + it('visually hides only the label when the heading has other content', async () => { + const element = await createField( + {label: 'My field', 'label-sr-only': ''}, + '' + ); + + const heading = element.shadowRoot!.querySelector('.form-field__label')!; + expect(heading.classList.contains('cp-visually-hidden')).toBe(false); + expect( + heading.querySelector('.cp-visually-hidden > slot[name="label"]') + ).not.toBeNull(); + }); + + it('shows the label again when label-sr-only is unset', async () => { + const element = await createField({ + label: 'My field', + 'label-sr-only': '', + }); + + element.labelSrOnly = false; + await element.updateComplete; + + expect(element.shadowRoot!.querySelector('.cp-visually-hidden')).toBeNull(); + }); +}); + describe('craft-field heading prefix/suffix', () => { it('renders heading-prefix and heading-suffix slots around the label', async () => { const element = document.createElement('craft-field') as CraftField; diff --git a/packages/craftcms-ui/src/components/field/field.ts b/packages/craftcms-ui/src/components/field/field.ts index 3cc8e931515..cc60bf8d0d2 100644 --- a/packages/craftcms-ui/src/components/field/field.ts +++ b/packages/craftcms-ui/src/components/field/field.ts @@ -1,4 +1,10 @@ -import {LitElement, html, nothing, type PropertyValues} from 'lit'; +import { + LitElement, + html, + nothing, + type PropertyValues, + type TemplateResult, +} from 'lit'; import {property} from 'lit/decorators.js'; import {classMap} from 'lit/directives/class-map.js'; import {ifDefined} from 'lit/directives/if-defined.js'; @@ -8,6 +14,7 @@ import '../callout/callout.js'; import {baseFieldStyles} from '@src/styles/form.styles'; import visuallyHiddenStyles from '@src/styles/visually-hidden.styles.js'; import styles from './field.styles.js'; +import {HasSlotController} from '@src/utilities/slot'; import {t} from '@src/utilities/translate'; type FormControlTarget = HTMLElement & { @@ -98,6 +105,23 @@ export default class CraftField extends FormControlMixin(LitElement) { */ @property({type: String, reflect: true}) width?: 'full' | 'auto'; + /** + * Visually hides the label, keeping it available to screen readers. + * `FormControlMixin` declares this property (`label-sr-only`), but its + * types leave it out. + */ + declare labelSrOnly: boolean; + + private readonly __hasSlot = new HasSlotController( + this, + 'heading-prefix', + 'label-extra', + 'actions', + 'heading-suffix', + 'tip', + 'warning' + ); + private __lightDomObserver = new MutationObserver(() => this.__onLightDomChanged() ); @@ -306,19 +330,53 @@ export default class CraftField extends FormControlMixin(LitElement) { /** * The field heading: label, read-only badge, flex-grow spacer, label extras - * and actions, mirroring `.field > .heading` in the Blade wrapper. + * and actions, mirroring `.field > .heading` in the Blade wrapper. Skipped + * entirely when there's nothing to put in it. + * + * With `label-sr-only`, the label stays available to screen readers but is + * visually hidden. When it's the only thing in the heading, the whole + * heading is hidden rather than just the label, so it leaves the flow and + * takes no gap; otherwise only the label is, keeping the rest visible. */ protected override _labelTemplate() { - const hasActions = this.__hasLightChild('actions'); + const hasLabel = this.__hasLabel; + const hasLabelExtra = this.__hasSlot.test('label-extra'); + const hasActions = this.__hasSlot.test('actions'); + const hasOtherContent = + hasLabelExtra || + hasActions || + this.readOnly || + this.__hasSlot.test('heading-prefix') || + this.__hasSlot.test('heading-suffix'); + + if (!hasLabel && !hasOtherContent) { + // Lion types this override as returning a TemplateResult. + return html``; + } + + let label: TemplateResult | typeof nothing = nothing; + if (hasLabel) { + label = + this.labelSrOnly && hasOtherContent + ? html`` + : html``; + } return html` -
+
- + ${label} ${this.readOnly ? html`${t('Read Only')}` : nothing} - ${this.__hasLightChild('label-extra') || hasActions + ${hasLabelExtra || hasActions ? html`
` : nothing} @@ -339,6 +397,11 @@ export default class CraftField extends FormControlMixin(LitElement) { `; } + /** The instructions, skipped entirely when there are none. */ + protected override _helpTextTemplate() { + return this.__hasHelpText ? super._helpTextTemplate() : html``; + } + /** * The input container, mirroring the server-side * `.input.{orientation}.errors.disabled` classes while keeping Lion's @@ -385,7 +448,7 @@ export default class CraftField extends FormControlMixin(LitElement) { * for warnings, each with a visually hidden prefix. */ protected _noticeTemplate(kind: 'tip' | 'warning') { - if (!this.__hasLightChild(kind)) { + if (!this.__hasSlot.test(kind)) { return nothing; } @@ -406,8 +469,19 @@ export default class CraftField extends FormControlMixin(LitElement) { `; } - private __hasLightChild(slotName: string): boolean { - return this.__lightChild(slotName) !== undefined; + /** + * Whether there's a label to show. Lion's `SlotMixin` always generates a + * light-DOM label node (empty when there's no `label`), so the node's + * presence says nothing — its text does. `label` reads that text back when + * a consumer slots their own label instead of setting the attribute. + */ + private get __hasLabel(): boolean { + return this.label.trim() !== ''; + } + + /** Whether there are instructions to show. See `__hasLabel`. */ + private get __hasHelpText(): boolean { + return this.helpText.trim() !== ''; } private __lightChild(slotName: string): HTMLElement | undefined { @@ -423,8 +497,8 @@ export default class CraftField extends FormControlMixin(LitElement) { this.__syncLabelDecorations(); this.__syncHasMaxlength(); this.__syncControlWidth(); - // Conditional templates (tip/warning callouts, heading spacer, action - // group) depend on light DOM children. + // Conditional templates (label, instructions, tip/warning callouts, + // heading spacer, action group) depend on light DOM children. this.requestUpdate(); } diff --git a/packages/craftcms-ui/src/styles/form.styles.ts b/packages/craftcms-ui/src/styles/form.styles.ts index 0f1aec7d702..28570befc85 100644 --- a/packages/craftcms-ui/src/styles/form.styles.ts +++ b/packages/craftcms-ui/src/styles/form.styles.ts @@ -10,6 +10,10 @@ import {css} from 'lit'; * min-height, so the control's `:focus-visible` outline traces its own shape * rather than an ancestor's. Pair it with {@link baseInputWrapperStyles} on * the surrounding `.input-group__input`/`.input-group__container` div. + * + * The control is sized border-box, so `--c-input-height` is its full outer + * height — matching buttons — whether or not the page applies a global + * border-box reset. */ export const baseFormControlStyles = css` --_input-border-width: var( @@ -27,8 +31,8 @@ export const baseFormControlStyles = css` var(--_input-end-end-radius) var(--_input-end-start-radius); background-color: var(--c-input-fill, var(--c-form-control-fill)); box-shadow: var(--c-input-shadow); - min-height: var(--c-input-height, var(--c-size-control-md)); box-sizing: border-box; + min-height: var(--c-input-height, var(--c-size-control-md)); `; /** @@ -75,7 +79,6 @@ export const baseComboboxStyles = css` ${baseFormControlStyles} width: 100%; height: 100%; - min-height: none; appearance: none; padding-inline: var(--c-input-spacing-inline) calc(var(--c-input-spacing-inline) * 1.5 + 1em); diff --git a/packages/craftcms-ui/src/utilities/slot.test.ts b/packages/craftcms-ui/src/utilities/slot.test.ts new file mode 100644 index 00000000000..97551f5e58b --- /dev/null +++ b/packages/craftcms-ui/src/utilities/slot.test.ts @@ -0,0 +1,118 @@ +import {LitElement, html} from 'lit'; +import {afterEach, describe, expect, it} from 'vite-plus/test'; +import {HasSlotController} from './slot'; + +class SlotHost extends LitElement { + readonly hasSlot = new HasSlotController( + this, + HasSlotController.DEFAULT_SLOT, + 'footer' + ); + + renderCount = 0; + + override render() { + this.renderCount++; + return html``; + } +} + +customElements.define('test-slot-host', SlotHost); + +async function createHost(innerHTML = ''): Promise { + const host = document.createElement('test-slot-host') as SlotHost; + host.innerHTML = innerHTML; + document.body.append(host); + await host.updateComplete; + return host; +} + +/** Lets the `MutationObserver` deliver, then the host re-render. */ +async function settle(host: SlotHost): Promise { + await new Promise((resolve) => setTimeout(resolve)); + await host.updateComplete; +} + +afterEach(() => { + document.body.innerHTML = ''; +}); + +describe('HasSlotController', () => { + it('detects a named slot', async () => { + const host = await createHost('
Footer
'); + expect(host.hasSlot.test('footer')).toBe(true); + }); + + it('reports an empty named slot', async () => { + const host = await createHost('
Other
'); + expect(host.hasSlot.test('footer')).toBe(false); + }); + + it('only counts direct children', async () => { + const host = await createHost( + '
Nested
' + ); + expect(host.hasSlot.test('footer')).toBe(false); + }); + + it('detects default slot text and elements', async () => { + expect((await createHost('Text')).hasSlot.test('[default]')).toBe(true); + expect((await createHost('

Text

')).hasSlot.test('[default]')).toBe( + true + ); + }); + + it('ignores whitespace and named slot content for the default slot', async () => { + const host = await createHost('
Footer
'); + expect(host.hasSlot.test(HasSlotController.DEFAULT_SLOT)).toBe(false); + }); + + it('re-renders when a tracked slot gains or loses content', async () => { + const host = await createHost(); + const rendersBefore = host.renderCount; + + const footer = document.createElement('div'); + footer.slot = 'footer'; + host.append(footer); + await settle(host); + expect(host.hasSlot.test('footer')).toBe(true); + expect(host.renderCount).toBe(rendersBefore + 1); + + footer.remove(); + await settle(host); + expect(host.hasSlot.test('footer')).toBe(false); + expect(host.renderCount).toBe(rendersBefore + 2); + }); + + it('re-renders when a child is re-slotted in place', async () => { + const host = await createHost('
Footer
'); + const rendersBefore = host.renderCount; + + host.querySelector('div')!.slot = 'footer'; + await settle(host); + + expect(host.renderCount).toBe(rendersBefore + 1); + }); + + it('does not re-render when presence is unchanged', async () => { + const host = await createHost('
Footer
'); + const rendersBefore = host.renderCount; + + host.querySelector('div')!.textContent = 'Updated'; + host.append(Object.assign(document.createElement('div'), {slot: 'other'})); + await settle(host); + + expect(host.renderCount).toBe(rendersBefore); + }); + + it('stops observing once disconnected', async () => { + const host = await createHost(); + host.remove(); + const rendersBefore = host.renderCount; + + host.append(Object.assign(document.createElement('div'), {slot: 'footer'})); + await settle(host); + + expect(host.renderCount).toBe(rendersBefore); + }); +}); diff --git a/packages/craftcms-ui/src/utilities/slot.ts b/packages/craftcms-ui/src/utilities/slot.ts new file mode 100644 index 00000000000..cee0165126c --- /dev/null +++ b/packages/craftcms-ui/src/utilities/slot.ts @@ -0,0 +1,104 @@ +import type {ReactiveController, ReactiveControllerHost} from 'lit'; + +/** + * A reactive controller that tracks whether a host's slots have content, so + * the host can skip rendering a slot's wrapper when nothing is slotted into + * it. Modeled on Web Awesome's `HasSlotController`. + * + * Presence is read from the host's direct light-DOM children, and a light-DOM + * `MutationObserver` re-renders the host when a tracked slot gains or loses + * content. `slotchange` alone isn't enough: a slot that isn't rendered never + * fires one, so it could never come back. + * + * @example + * ```ts + * private readonly hasSlot = new HasSlotController(this, 'footer'); + * + * render() { + * return this.hasSlot.test('footer') + * ? html`
` + * : nothing; + * } + * ``` + */ +export class HasSlotController implements ReactiveController { + /** Pass as a slot name to track the default (unnamed) slot. */ + static readonly DEFAULT_SLOT = '[default]'; + + private readonly slotNames: string[]; + + private observer?: MutationObserver; + + private presence: boolean[] = []; + + constructor( + private readonly host: ReactiveControllerHost & HTMLElement, + ...slotNames: string[] + ) { + this.slotNames = slotNames; + host.addController(this); + } + + /** + * Whether anything is assigned to the given slot. Pass + * `HasSlotController.DEFAULT_SLOT` to test the default slot. + */ + test(slotName: string): boolean { + return slotName === HasSlotController.DEFAULT_SLOT + ? this.hasDefaultSlot() + : this.hasNamedSlot(slotName); + } + + hostConnected(): void { + this.presence = this.snapshot(); + // Created here rather than in the constructor: hosts are also + // constructed during SSR, where `MutationObserver` doesn't exist. + this.observer ??= new MutationObserver(this.handleMutations); + this.observer.observe(this.host, { + childList: true, + // Children re-slotted in place (a changed `slot` attribute). + subtree: true, + attributes: true, + attributeFilter: ['slot'], + // Default-slot presence depends on direct text nodes' content. + characterData: this.slotNames.includes(HasSlotController.DEFAULT_SLOT), + }); + } + + hostDisconnected(): void { + this.observer?.disconnect(); + } + + private hasDefaultSlot(): boolean { + return Array.from(this.host.childNodes).some((node) => { + if (node.nodeType === Node.TEXT_NODE) { + return node.textContent!.trim() !== ''; + } + + return node.nodeType === Node.ELEMENT_NODE && !(node as Element).slot; + }); + } + + private hasNamedSlot(slotName: string): boolean { + return Array.from(this.host.children).some( + (child) => child.slot === slotName + ); + } + + private snapshot(): boolean[] { + return this.slotNames.map((slotName) => this.test(slotName)); + } + + /** + * Re-renders only when a tracked slot's presence actually flips, so + * unrelated light-DOM churn (including deep subtree mutations) is cheap. + */ + private handleMutations = (): void => { + const presence = this.snapshot(); + + if (presence.some((hasSlot, index) => hasSlot !== this.presence[index])) { + this.presence = presence; + this.host.requestUpdate(); + } + }; +} diff --git a/resources/js/modules/conditions/ConditionBuilder.test.ts b/resources/js/modules/conditions/ConditionBuilder.test.ts index c66ee97b035..ed55d898f7e 100644 --- a/resources/js/modules/conditions/ConditionBuilder.test.ts +++ b/resources/js/modules/conditions/ConditionBuilder.test.ts @@ -188,6 +188,17 @@ async function changeOperator(): Promise { await nextTick(); } +async function changeGroupOperator( + value: GroupConfig['operator'] +): Promise { + const operator = container.querySelector( + '.condition-group__operator select' + )!; + operator.value = value; + operator.dispatchEvent(new Event('change', {bubbles: true})); + await nextTick(); +} + function submitted(): GroupConfig { return ( expandFormData(new FormData(form)).condition as { @@ -199,7 +210,7 @@ function submitted(): GroupConfig { it('edits nested operators locally and prunes empty groups only in submitted values', async () => { const post = vi.spyOn(actionClient, 'request'); await mount(); - button('Any').click(); + await changeGroupOperator('or'); button('Add a group').click(); await nextTick(); diff --git a/resources/js/modules/conditions/ConditionRule.vue b/resources/js/modules/conditions/ConditionRule.vue index c2043c1bf17..1feb2f8f898 100644 --- a/resources/js/modules/conditions/ConditionRule.vue +++ b/resources/js/modules/conditions/ConditionRule.vue @@ -169,8 +169,6 @@ flex: 0 0 auto; } - .condition-rule-fields - :deep(craft-field:not(:has(craft-input-date-time)) > [slot='label']), .condition-rule-fields :deep(craft-combobox > [slot='label']) { position: absolute; width: 1px; diff --git a/resources/js/modules/forms/ChoiceControl.vue b/resources/js/modules/forms/ChoiceControl.vue index ccc853c57ed..d27ede186e1 100644 --- a/resources/js/modules/forms/ChoiceControl.vue +++ b/resources/js/modules/forms/ChoiceControl.vue @@ -7,12 +7,12 @@ import '@craftcms/ui/components/radio/radio'; import '@craftcms/ui/components/radio-group/radio-group'; import '@craftcms/ui/components/select/select'; - import {computed, ref, watch} from 'vue'; + import {computed, inject, ref, watch} from 'vue'; import type {CheckboxOption} from '@/common/types'; import CheckboxGroup from '@/common/form/CheckboxGroup.vue'; import type {FormControlPayload, FormValue} from './types'; import type {Slots} from 'vue'; - import {inputName, serverErrorValidators} from './runtime'; + import {FieldLabelSrOnly, inputName, serverErrorValidators} from './runtime'; type ChoiceValue = boolean | number | string; type ChoicePresentation = CraftCms.Cms.Form.Enums.ChoicePresentation; @@ -62,6 +62,7 @@ const emit = defineEmits<{ (event: 'update:value', value: string | string[]): void; }>(); + const fieldLabelSrOnly = inject(FieldLabelSrOnly, undefined); /** * A single select needs somewhere to represent "nothing chosen". @@ -296,6 +297,7 @@