Skip to content

Commit 16ce3fd

Browse files
JeroenDeDauwclaude
andcommitted
Constrain the target-schema picker to existing schemas
The target schema is a free-text CdxLookup, so typed text could drift from the actual selection (e.g. "Produc" while "Product" stays selected) with no feedback, and that stale value could be saved by changing another field. Reconcile on blur: keep a schema picked from the menu, clear when the field is emptied, otherwise revert unmatched text to the committed value. The field can now only hold an existing schema or nothing. Filtering still uses the server-side prefix search. Browser-verified on Schema:Company (revert, pick-to-replace, clear). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 04432c9 commit 16ce3fd

2 files changed

Lines changed: 155 additions & 99 deletions

File tree

resources/ext.neowiki/src/components/common/SchemaLookup.vue

Lines changed: 21 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
:placeholder="$i18n( 'neowiki-subject-creator-schema-search-placeholder' ).text()"
1010
@input="onLookupInput"
1111
@update:selected="onSchemaSelected"
12+
@blur="commitOrRevert"
1213
/>
1314
</div>
1415
</template>
@@ -31,20 +32,13 @@ const emit = defineEmits<{
3132
const schemaStore = useSchemaStore();
3233
const selectedSchema = ref<string | null>( props.selected ?? null );
3334
const inputText = ref<string>( props.selected ?? '' );
34-
const menuItems = ref<MenuItemData[]>(
35-
props.selected ? [ { label: props.selected, value: props.selected } ] : []
36-
);
35+
const menuItems = ref<MenuItemData[]>( [] );
3736
const lookupRef = ref<InstanceType<typeof CdxLookup> | null>( null );
3837
let requestSequence = 0;
3938
4039
watch( () => props.selected, ( value ) => {
4140
selectedSchema.value = value ?? null;
4241
inputText.value = value ?? '';
43-
if ( value && !menuItems.value.some( ( item ) => item.value === value ) ) {
44-
menuItems.value = [ { label: value, value: value } ];
45-
} else if ( !value ) {
46-
menuItems.value = [];
47-
}
4842
} );
4943
5044
async function onLookupInput( value: string ): Promise<void> {
@@ -80,11 +74,28 @@ async function onLookupInput( value: string ): Promise<void> {
8074
function onSchemaSelected( schemaName: string | null ): void {
8175
if ( schemaName ) {
8276
emit( 'select', schemaName );
83-
} else if ( !inputText.value ) {
84-
emit( 'select', '' );
8577
}
8678
}
8779
80+
// The field may only hold a schema that was picked from the menu. CdxLookup nulls
81+
// its selection while the user types, so on blur reconcile the unconfirmed text:
82+
// clear when emptied, otherwise revert to the last committed value.
83+
function commitOrRevert(): void {
84+
if ( selectedSchema.value !== null ) {
85+
return;
86+
}
87+
88+
if ( inputText.value === '' ) {
89+
if ( props.selected ) {
90+
emit( 'select', '' );
91+
}
92+
return;
93+
}
94+
95+
inputText.value = props.selected ?? '';
96+
selectedSchema.value = props.selected ?? null;
97+
}
98+
8899
function focus(): void {
89100
// CdxLookup component does not expose a focus method,
90101
// so we need to find the input element and focus it directly.

resources/ext.neowiki/tests/components/common/SchemaLookup.spec.ts

Lines changed: 134 additions & 89 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { mount, VueWrapper, flushPromises } from '@vue/test-utils';
22
import { beforeEach, describe, expect, it, vi } from 'vitest';
3+
import { nextTick } from 'vue';
34
import SchemaLookup from '@/components/common/SchemaLookup.vue';
45
import { createPinia, setActivePinia } from 'pinia';
56
import { useSchemaStore } from '@/stores/SchemaStore.ts';
@@ -37,133 +38,177 @@ describe( 'SchemaLookup', () => {
3738
schemaStore.searchAndFetchMissingSchemas = vi.fn().mockResolvedValue( [] );
3839
} );
3940

40-
it( 'searches for schemas when input changes', () => {
41-
const wrapper = mountComponent();
42-
const lookup = wrapper.findComponent( CdxLookup );
41+
describe( 'searching', () => {
42+
it( 'searches for schemas when input changes', () => {
43+
const wrapper = mountComponent();
44+
const lookup = wrapper.findComponent( CdxLookup );
4345

44-
lookup.vm.$emit( 'input', 'test query' );
46+
lookup.vm.$emit( 'input', 'test query' );
4547

46-
expect( schemaStore.searchAndFetchMissingSchemas ).toHaveBeenCalledWith( 'test query' );
47-
} );
48-
49-
it( 'updates menu items with search results', async () => {
50-
const mockResults = [ 'Schema1', 'Schema2' ];
51-
schemaStore.searchAndFetchMissingSchemas.mockResolvedValue( mockResults );
52-
schemaStore.schemas.set( 'Schema1', new Schema( 'Schema1', 'First description', new PropertyDefinitionList( [] ) ) );
53-
schemaStore.schemas.set( 'Schema2', new Schema( 'Schema2', 'Second description', new PropertyDefinitionList( [] ) ) );
48+
expect( schemaStore.searchAndFetchMissingSchemas ).toHaveBeenCalledWith( 'test query' );
49+
} );
5450

55-
const wrapper = mountComponent();
56-
const lookup = wrapper.findComponent( CdxLookup );
51+
it( 'updates menu items with search results', async () => {
52+
schemaStore.searchAndFetchMissingSchemas.mockResolvedValue( [ 'Schema1', 'Schema2' ] );
53+
schemaStore.schemas.set( 'Schema1', new Schema( 'Schema1', 'First description', new PropertyDefinitionList( [] ) ) );
54+
schemaStore.schemas.set( 'Schema2', new Schema( 'Schema2', 'Second description', new PropertyDefinitionList( [] ) ) );
5755

58-
lookup.vm.$emit( 'input', 'test' );
59-
await flushPromises();
56+
const wrapper = mountComponent();
57+
const lookup = wrapper.findComponent( CdxLookup );
6058

61-
expect( lookup.props( 'menuItems' ) ).toEqual( [
62-
{ label: 'Schema1', value: 'Schema1', description: 'First description' },
63-
{ label: 'Schema2', value: 'Schema2', description: 'Second description' },
64-
] );
65-
} );
59+
lookup.vm.$emit( 'input', 'test' );
60+
await flushPromises();
6661

67-
it( 'omits description from menu items when schema has empty description', async () => {
68-
schemaStore.searchAndFetchMissingSchemas.mockResolvedValue( [ 'WithDesc', 'NoDesc' ] );
69-
schemaStore.schemas.set( 'WithDesc', new Schema( 'WithDesc', 'Has a description', new PropertyDefinitionList( [] ) ) );
70-
schemaStore.schemas.set( 'NoDesc', new Schema( 'NoDesc', '', new PropertyDefinitionList( [] ) ) );
62+
expect( lookup.props( 'menuItems' ) ).toEqual( [
63+
{ label: 'Schema1', value: 'Schema1', description: 'First description' },
64+
{ label: 'Schema2', value: 'Schema2', description: 'Second description' },
65+
] );
66+
} );
7167

72-
const wrapper = mountComponent();
73-
const lookup = wrapper.findComponent( CdxLookup );
68+
it( 'omits description from menu items when schema has empty description', async () => {
69+
schemaStore.searchAndFetchMissingSchemas.mockResolvedValue( [ 'WithDesc', 'NoDesc' ] );
70+
schemaStore.schemas.set( 'WithDesc', new Schema( 'WithDesc', 'Has a description', new PropertyDefinitionList( [] ) ) );
71+
schemaStore.schemas.set( 'NoDesc', new Schema( 'NoDesc', '', new PropertyDefinitionList( [] ) ) );
7472

75-
lookup.vm.$emit( 'input', 'test' );
76-
await flushPromises();
73+
const wrapper = mountComponent();
74+
const lookup = wrapper.findComponent( CdxLookup );
7775

78-
expect( lookup.props( 'menuItems' ) ).toEqual( [
79-
{ label: 'WithDesc', value: 'WithDesc', description: 'Has a description' },
80-
{ label: 'NoDesc', value: 'NoDesc', description: undefined },
81-
] );
82-
} );
76+
lookup.vm.$emit( 'input', 'test' );
77+
await flushPromises();
8378

84-
it( 'discards stale search results when a newer request completes first', async () => {
85-
let resolveFirst: ( value: string[] ) => void;
86-
const firstCallPromise = new Promise<string[]>( ( resolve ) => {
87-
resolveFirst = resolve;
79+
expect( lookup.props( 'menuItems' ) ).toEqual( [
80+
{ label: 'WithDesc', value: 'WithDesc', description: 'Has a description' },
81+
{ label: 'NoDesc', value: 'NoDesc', description: undefined },
82+
] );
8883
} );
8984

90-
schemaStore.searchAndFetchMissingSchemas = vi.fn()
91-
.mockReturnValueOnce( firstCallPromise )
92-
.mockResolvedValueOnce( [ 'SecondSchema' ] );
85+
it( 'discards stale search results when a newer request completes first', async () => {
86+
let resolveFirst: ( value: string[] ) => void;
87+
const firstCallPromise = new Promise<string[]>( ( resolve ) => {
88+
resolveFirst = resolve;
89+
} );
90+
91+
schemaStore.searchAndFetchMissingSchemas = vi.fn()
92+
.mockReturnValueOnce( firstCallPromise )
93+
.mockResolvedValueOnce( [ 'SecondSchema' ] );
9394

94-
schemaStore.schemas.set( 'FirstSchema', new Schema( 'FirstSchema', 'Stale', new PropertyDefinitionList( [] ) ) );
95-
schemaStore.schemas.set( 'SecondSchema', new Schema( 'SecondSchema', 'Fresh', new PropertyDefinitionList( [] ) ) );
95+
schemaStore.schemas.set( 'FirstSchema', new Schema( 'FirstSchema', 'Stale', new PropertyDefinitionList( [] ) ) );
96+
schemaStore.schemas.set( 'SecondSchema', new Schema( 'SecondSchema', 'Fresh', new PropertyDefinitionList( [] ) ) );
9697

97-
const wrapper = mountComponent();
98-
const lookup = wrapper.findComponent( CdxLookup );
98+
const wrapper = mountComponent();
99+
const lookup = wrapper.findComponent( CdxLookup );
99100

100-
lookup.vm.$emit( 'input', 'first' );
101-
lookup.vm.$emit( 'input', 'second' );
102-
await flushPromises();
101+
lookup.vm.$emit( 'input', 'first' );
102+
lookup.vm.$emit( 'input', 'second' );
103+
await flushPromises();
103104

104-
expect( lookup.props( 'menuItems' ) ).toEqual( [
105-
{ label: 'SecondSchema', value: 'SecondSchema', description: 'Fresh' },
106-
] );
105+
expect( lookup.props( 'menuItems' ) ).toEqual( [
106+
{ label: 'SecondSchema', value: 'SecondSchema', description: 'Fresh' },
107+
] );
107108

108-
resolveFirst!( [ 'FirstSchema' ] );
109-
await flushPromises();
109+
resolveFirst!( [ 'FirstSchema' ] );
110+
await flushPromises();
110111

111-
expect( lookup.props( 'menuItems' ) ).toEqual( [
112-
{ label: 'SecondSchema', value: 'SecondSchema', description: 'Fresh' },
113-
] );
112+
expect( lookup.props( 'menuItems' ) ).toEqual( [
113+
{ label: 'SecondSchema', value: 'SecondSchema', description: 'Fresh' },
114+
] );
115+
} );
114116
} );
115117

116-
it( 'reflects the selected prop on the lookup', () => {
117-
const wrapper = mountComponent( { selected: 'Product' } );
118-
const lookup = wrapper.findComponent( CdxLookup );
118+
describe( 'committing a selection', () => {
119+
it( 'emits the chosen schema when one is selected', () => {
120+
const wrapper = mountComponent();
121+
const lookup = wrapper.findComponent( CdxLookup );
119122

120-
expect( lookup.props( 'selected' ) ).toBe( 'Product' );
121-
expect( lookup.props( 'inputValue' ) ).toBe( 'Product' );
122-
expect( lookup.props( 'menuItems' ) ).toEqual( [ { label: 'Product', value: 'Product' } ] );
123-
} );
123+
lookup.vm.$emit( 'update:selected', 'Product' );
124+
125+
expect( wrapper.emitted( 'select' )?.[ 0 ] ).toEqual( [ 'Product' ] );
126+
} );
127+
128+
it( 'does not emit while the selection is being changed by typing', () => {
129+
const wrapper = mountComponent( { selected: 'Product' } );
130+
const lookup = wrapper.findComponent( CdxLookup );
124131

125-
it( 'updates the lookup when the selected prop changes after mount', async () => {
126-
const wrapper = mountComponent();
127-
const lookup = wrapper.findComponent( CdxLookup );
132+
lookup.vm.$emit( 'update:input-value', 'Off' );
133+
lookup.vm.$emit( 'update:selected', null );
128134

129-
await wrapper.setProps( { selected: 'NewSchema' } );
135+
expect( wrapper.emitted( 'select' ) ).toBeFalsy();
136+
} );
130137

131-
expect( lookup.props( 'selected' ) ).toBe( 'NewSchema' );
132-
expect( lookup.props( 'inputValue' ) ).toBe( 'NewSchema' );
133-
expect( lookup.props( 'menuItems' ) ).toEqual( [ { label: 'NewSchema', value: 'NewSchema' } ] );
138+
it( 'keeps a chosen schema untouched on blur', () => {
139+
const wrapper = mountComponent( { selected: 'Product' } );
140+
const lookup = wrapper.findComponent( CdxLookup );
134141

135-
await wrapper.setProps( { selected: null } );
142+
lookup.vm.$emit( 'update:selected', 'Office' );
143+
lookup.vm.$emit( 'update:input-value', 'Office' );
144+
lookup.vm.$emit( 'blur' );
136145

137-
expect( lookup.props( 'inputValue' ) ).toBe( '' );
138-
expect( lookup.props( 'menuItems' ) ).toEqual( [] );
146+
expect( wrapper.emitted( 'select' ) ).toEqual( [ [ 'Office' ] ] );
147+
} );
139148
} );
140149

141-
it( 'emits the selected schema when one is chosen', () => {
142-
const wrapper = mountComponent();
143-
const lookup = wrapper.findComponent( CdxLookup );
150+
describe( 'rejecting invalid input', () => {
151+
it( 'reverts unmatched typed text to the committed schema on blur', async () => {
152+
const wrapper = mountComponent( { selected: 'Product' } );
153+
const lookup = wrapper.findComponent( CdxLookup );
144154

145-
lookup.vm.$emit( 'update:selected', 'Product' );
155+
lookup.vm.$emit( 'update:selected', null );
156+
lookup.vm.$emit( 'update:input-value', 'Produc' );
157+
lookup.vm.$emit( 'blur' );
158+
await nextTick();
146159

147-
expect( wrapper.emitted( 'select' )?.[ 0 ] ).toEqual( [ 'Product' ] );
160+
expect( lookup.props( 'inputValue' ) ).toBe( 'Product' );
161+
expect( wrapper.emitted( 'select' ) ).toBeFalsy();
162+
} );
148163
} );
149164

150-
it( 'emits an empty selection when the lookup is cleared', () => {
151-
const wrapper = mountComponent();
152-
const lookup = wrapper.findComponent( CdxLookup );
165+
describe( 'clearing', () => {
166+
it( 'clears the selection when the field is emptied and blurred', () => {
167+
const wrapper = mountComponent( { selected: 'Product' } );
168+
const lookup = wrapper.findComponent( CdxLookup );
153169

154-
lookup.vm.$emit( 'update:selected', null );
170+
lookup.vm.$emit( 'update:selected', null );
171+
lookup.vm.$emit( 'update:input-value', '' );
172+
lookup.vm.$emit( 'blur' );
173+
174+
expect( wrapper.emitted( 'select' )?.[ 0 ] ).toEqual( [ '' ] );
175+
} );
155176

156-
expect( wrapper.emitted( 'select' )?.[ 0 ] ).toEqual( [ '' ] );
177+
it( 'does not emit a clear when an unset field is blurred', () => {
178+
const wrapper = mountComponent();
179+
const lookup = wrapper.findComponent( CdxLookup );
180+
181+
lookup.vm.$emit( 'update:selected', null );
182+
lookup.vm.$emit( 'update:input-value', '' );
183+
lookup.vm.$emit( 'blur' );
184+
185+
expect( wrapper.emitted( 'select' ) ).toBeFalsy();
186+
} );
157187
} );
158188

159-
it( 'does not emit a clear while the user is typing a replacement', () => {
160-
const wrapper = mountComponent( { selected: 'Product' } );
161-
const lookup = wrapper.findComponent( CdxLookup );
189+
describe( 'reflecting the selected prop', () => {
190+
it( 'shows the selected schema in the field', () => {
191+
const wrapper = mountComponent( { selected: 'Product' } );
192+
const lookup = wrapper.findComponent( CdxLookup );
162193

163-
lookup.vm.$emit( 'update:input-value', 'Off' );
164-
lookup.vm.$emit( 'update:selected', null );
194+
expect( lookup.props( 'selected' ) ).toBe( 'Product' );
195+
expect( lookup.props( 'inputValue' ) ).toBe( 'Product' );
196+
} );
197+
198+
it( 'updates the field when the selected prop changes', async () => {
199+
const wrapper = mountComponent();
200+
const lookup = wrapper.findComponent( CdxLookup );
201+
202+
await wrapper.setProps( { selected: 'NewSchema' } );
165203

166-
expect( wrapper.emitted( 'select' ) ).toBeFalsy();
204+
expect( lookup.props( 'selected' ) ).toBe( 'NewSchema' );
205+
expect( lookup.props( 'inputValue' ) ).toBe( 'NewSchema' );
206+
207+
await wrapper.setProps( { selected: null } );
208+
209+
expect( lookup.props( 'selected' ) ).toBe( null );
210+
expect( lookup.props( 'inputValue' ) ).toBe( '' );
211+
} );
167212
} );
168213

169214
it( 'exposes focus method', () => {

0 commit comments

Comments
 (0)