Skip to content

Commit 1a5f57c

Browse files
malbertsclaude
malberts
and
claude
authored
Boolean property: use CdxCheckbox instead of CdxToggleSwitch (#850)
* Boolean property: use CdxCheckbox instead of CdxToggleSwitch Codex's ToggleSwitch usage guideline says: Use the ToggleSwitch component only where an instant change in the user-interface based on their assigned action is intended. For non-instant selections and selection groups, use Checkbox instead. BooleanInput is used in two non-instant contexts: - Subject editor: the value lands in a form gated by Save. - Schema editor's "Initial value" control: also lives inside the property-definition form, gated by Save. Both fit the "non-instant selection" case, so Checkbox is the guideline-correct widget. Local to BooleanInput.vue: same :model-value / @update:model-value wiring; the toggle's :align-switch and :label props are dropped (no Checkbox equivalents), the label text and optional description icon move into the Checkbox default slot. No data-shape change: BooleanInput still emits BooleanValue(true/false) on change, still renders checked iff modelValue.boolean === true, and still defaults to false for undefined / wrong-type input (the "Boolean is never unset" choice from PR #837). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * BooleanInput: field heading and inline label match the call site Shape field labelling to fit both call sites: - Inline checkbox label is the property's own name (e.g. "Is public"). This is what the subject editor showed before the swap and what users see when they toggle the value. - The CdxField #label heading shows the caller-supplied props.label, but only when it differs from the property name. The schema editor passes "Initial value" so the heading renders and clarifies what the bare checkbox at the bottom of the property-definition form represents (the default for new subjects, not a value on the property itself). The subject editor passes the property name as the label, so the heading would just repeat the inline checkbox label; suppressed. - :optional is not set on the CdxField, because a Boolean value is always defined, so an "(optional)" affordance would be misleading. A short template comment documents both the heading-rule and the :optional omission so they don't read as oversights. Tests grow by two cases pinning both heading branches (shown when label differs from property name, hidden when they match). The fixture now sets a stable property name "Is public" so the inline label has a meaningful assertion target. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 16e7974 commit 1a5f57c

2 files changed

Lines changed: 52 additions & 29 deletions

File tree

‎resources/ext.neowiki/src/components/Value/BooleanInput.vue‎

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2,23 +2,27 @@
22
<CdxField
33
:status="validationError === null ? 'default' : 'error'"
44
:messages="validationError === null ? {} : { error: validationError }"
5-
:hide-label="true"
5+
:hide-label="!showFieldHeading"
66
>
7-
<CdxToggleSwitch
7+
<template
8+
v-if="showFieldHeading"
9+
#label
10+
>
11+
{{ props.label }}
12+
</template>
13+
<CdxCheckbox
814
:model-value="internalValue"
9-
:align-switch="true"
10-
:label="props.label"
1115
@update:model-value="onInput"
1216
>
13-
{{ props.label }}
17+
{{ props.property.name.toString() }}
1418
<CdxIcon
1519
v-if="props.property.description"
1620
v-tooltip="props.property.description"
1721
:icon="cdxIconInfo"
1822
class="ext-neowiki-value-input__description-icon"
1923
size="small"
2024
/>
21-
</CdxToggleSwitch>
25+
</CdxCheckbox>
2226
</CdxField>
2327
</template>
2428

@@ -27,8 +31,8 @@ import type { Value } from '@/domain/Value';
2731
</script>
2832

2933
<script setup lang="ts">
30-
import { ref, watch } from 'vue';
31-
import { CdxField, CdxIcon, CdxToggleSwitch } from '@wikimedia/codex';
34+
import { computed, ref, watch } from 'vue';
35+
import { CdxCheckbox, CdxField, CdxIcon } from '@wikimedia/codex';
3236
import { cdxIconInfo } from '@wikimedia/codex-icons';
3337
import { newBooleanValue, BooleanValue, ValueType } from '@/domain/Value';
3438
import { BooleanType, BooleanProperty } from '@/domain/propertyTypes/Boolean.ts';
@@ -47,6 +51,10 @@ const emit = defineEmits<ValueInputEmits>();
4751
4852
const validationError = ref<string | null>( null );
4953
54+
// Hide the heading when it would duplicate the inline checkbox label
55+
// (subject-editor case: the caller passes the property name as the label).
56+
const showFieldHeading = computed( () => props.label !== props.property.name.toString() );
57+
5058
const toBoolean = ( value: Value | undefined ): boolean =>
5159
value !== undefined && value.type === ValueType.Boolean ? ( value as BooleanValue ).boolean : false;
5260

‎resources/ext.neowiki/tests/components/Value/BooleanInput.spec.ts‎

Lines changed: 36 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { VueWrapper } from '@vue/test-utils';
22
import { beforeEach, describe, expect, it, vi } from 'vitest';
3-
import { CdxField, CdxToggleSwitch } from '@wikimedia/codex';
3+
import { CdxCheckbox, CdxField } from '@wikimedia/codex';
44
import { newBooleanValue, newStringValue, ValueType } from '@/domain/Value';
55
import BooleanInput from '@/components/Value/BooleanInput.vue';
66
import { newBooleanProperty, BooleanProperty } from '@/domain/propertyTypes/Boolean';
@@ -21,7 +21,7 @@ describe( 'BooleanInput', () => {
2121
return createTestWrapper( BooleanInput, {
2222
modelValue: undefined,
2323
label: 'Test Label',
24-
property: newBooleanProperty( {} ),
24+
property: newBooleanProperty( { name: 'Is public' } ),
2525
...props,
2626
} );
2727
}
@@ -30,50 +30,65 @@ describe( 'BooleanInput', () => {
3030
return ( wrapper.vm as unknown as ValueInputExposes ).getCurrentValue();
3131
}
3232

33-
it( 'renders a CdxToggleSwitch wrapped in a CdxField', () => {
33+
it( 'renders a CdxCheckbox wrapped in a CdxField, inline-labelled with the property name', () => {
3434
const wrapper = newWrapper();
3535

3636
expect( wrapper.findComponent( CdxField ).exists() ).toBe( true );
37-
expect( wrapper.findComponent( CdxToggleSwitch ).exists() ).toBe( true );
38-
expect( wrapper.text() ).toContain( 'Test Label' );
37+
expect( wrapper.findComponent( CdxCheckbox ).exists() ).toBe( true );
38+
expect( wrapper.findComponent( CdxCheckbox ).text() ).toContain( 'Is public' );
3939
} );
4040

41-
it( 'renders the toggle in the off position when modelValue is BooleanValue(false)', () => {
41+
it( 'shows the caller-supplied label as the field heading when it differs from the property name (schema-editor case)', () => {
42+
const wrapper = newWrapper( { label: 'Initial value' } );
43+
44+
expect( wrapper.findComponent( CdxField ).props( 'hideLabel' ) ).toBe( false );
45+
expect( wrapper.text() ).toContain( 'Initial value' );
46+
expect( wrapper.findComponent( CdxCheckbox ).text() ).toContain( 'Is public' );
47+
} );
48+
49+
it( 'hides the field heading when the caller-supplied label equals the property name (subject-editor case)', () => {
50+
const wrapper = newWrapper( { label: 'Is public' } );
51+
52+
expect( wrapper.findComponent( CdxField ).props( 'hideLabel' ) ).toBe( true );
53+
expect( wrapper.findComponent( CdxCheckbox ).text() ).toContain( 'Is public' );
54+
} );
55+
56+
it( 'renders the checkbox unchecked when modelValue is BooleanValue(false)', () => {
4257
const wrapper = newWrapper( { modelValue: newBooleanValue( false ) } );
4358

44-
expect( wrapper.findComponent( CdxToggleSwitch ).props( 'modelValue' ) ).toBe( false );
59+
expect( wrapper.findComponent( CdxCheckbox ).props( 'modelValue' ) ).toBe( false );
4560
} );
4661

47-
it( 'renders the toggle in the on position when modelValue is BooleanValue(true)', () => {
62+
it( 'renders the checkbox checked when modelValue is BooleanValue(true)', () => {
4863
const wrapper = newWrapper( { modelValue: newBooleanValue( true ) } );
4964

50-
expect( wrapper.findComponent( CdxToggleSwitch ).props( 'modelValue' ) ).toBe( true );
65+
expect( wrapper.findComponent( CdxCheckbox ).props( 'modelValue' ) ).toBe( true );
5166
} );
5267

53-
it( 'renders the toggle in the off position when modelValue is undefined', () => {
68+
it( 'renders the checkbox unchecked when modelValue is undefined', () => {
5469
const wrapper = newWrapper( { modelValue: undefined } );
5570

56-
expect( wrapper.findComponent( CdxToggleSwitch ).props( 'modelValue' ) ).toBe( false );
71+
expect( wrapper.findComponent( CdxCheckbox ).props( 'modelValue' ) ).toBe( false );
5772
} );
5873

59-
it( 'falls back to the off position when modelValue is the wrong value type', () => {
74+
it( 'falls back to unchecked when modelValue is the wrong value type', () => {
6075
const wrapper = newWrapper( { modelValue: newStringValue( 'not a boolean' ) } );
6176

62-
expect( wrapper.findComponent( CdxToggleSwitch ).props( 'modelValue' ) ).toBe( false );
77+
expect( wrapper.findComponent( CdxCheckbox ).props( 'modelValue' ) ).toBe( false );
6378
} );
6479

65-
it( 'emits BooleanValue(true) when the toggle is switched on', async () => {
80+
it( 'emits BooleanValue(true) when the checkbox is checked', async () => {
6681
const wrapper = newWrapper();
6782

68-
await wrapper.findComponent( CdxToggleSwitch ).vm.$emit( 'update:modelValue', true );
83+
await wrapper.findComponent( CdxCheckbox ).vm.$emit( 'update:modelValue', true );
6984

7085
expect( wrapper.emitted( 'update:modelValue' )?.[ 0 ] ).toEqual( [ newBooleanValue( true ) ] );
7186
} );
7287

73-
it( 'emits BooleanValue(false) when the toggle is switched off', async () => {
88+
it( 'emits BooleanValue(false) when the checkbox is unchecked', async () => {
7489
const wrapper = newWrapper( { modelValue: newBooleanValue( true ) } );
7590

76-
await wrapper.findComponent( CdxToggleSwitch ).vm.$emit( 'update:modelValue', false );
91+
await wrapper.findComponent( CdxCheckbox ).vm.$emit( 'update:modelValue', false );
7792

7893
expect( wrapper.emitted( 'update:modelValue' )?.[ 0 ] ).toEqual( [ newBooleanValue( false ) ] );
7994
} );
@@ -83,7 +98,7 @@ describe( 'BooleanInput', () => {
8398

8499
await wrapper.setProps( { modelValue: newBooleanValue( true ) } );
85100

86-
expect( wrapper.findComponent( CdxToggleSwitch ).props( 'modelValue' ) ).toBe( true );
101+
expect( wrapper.findComponent( CdxCheckbox ).props( 'modelValue' ) ).toBe( true );
87102
} );
88103

89104
describe( 'getCurrentValue', () => {
@@ -108,10 +123,10 @@ describe( 'BooleanInput', () => {
108123
expect( getCurrentValue( wrapper ) ).toEqual( newBooleanValue( false ) );
109124
} );
110125

111-
it( 'returns the toggled value after the user switches the toggle on', async () => {
126+
it( 'returns the new value after the user checks the checkbox', async () => {
112127
const wrapper = newWrapper( { modelValue: newBooleanValue( false ) } );
113128

114-
await wrapper.findComponent( CdxToggleSwitch ).vm.$emit( 'update:modelValue', true );
129+
await wrapper.findComponent( CdxCheckbox ).vm.$emit( 'update:modelValue', true );
115130

116131
expect( getCurrentValue( wrapper ) ).toEqual( newBooleanValue( true ) );
117132
} );
@@ -120,7 +135,7 @@ describe( 'BooleanInput', () => {
120135
it( 'emits a Value of type Boolean (not String or Number)', async () => {
121136
const wrapper = newWrapper();
122137

123-
await wrapper.findComponent( CdxToggleSwitch ).vm.$emit( 'update:modelValue', true );
138+
await wrapper.findComponent( CdxCheckbox ).vm.$emit( 'update:modelValue', true );
124139

125140
const events = wrapper.emitted( 'update:modelValue' );
126141
const emitted = events?.[ 0 ]?.[ 0 ] as { type: ValueType } | undefined;

0 commit comments

Comments
 (0)