Merge pull request #3745 from 73nici/SelectBoxInvalidValues

BUGFIX: Select box shows invalid value

Resources/Private/Translations/de/Main.xlf

@@ -630,6 +630,10 @@
         <source>Copy node type to clipboard</source>
         <target state="translated">Knotentyp in die Zwischenablage kopieren</target>
       </trans-unit>
+      <trans-unit id="invalidValue" xml:space="preserve">
+        <source>Invalid value</source>
+        <target state="translated">Ungültiger Wert</target>
+      </trans-unit>
     </body>
   </file>
 </xliff>

Resources/Private/Translations/en/Main.xlf

@@ -377,6 +377,9 @@
             <trans-unit id="copyNodeTypeNameToClipboard" xml:space="preserve">
                 <source>Copy node type to clipboard</source>
             </trans-unit>
+            <trans-unit id="invalidValue" xml:space="preserve">
+                <source>Invalid value</source>
+            </trans-unit>
         </body>
     </file>
 </xliff>

packages/neos-ui-editors/src/Editors/SelectBox/DataSourceBasedSelectBoxEditor.js

@@ -5,7 +5,7 @@ import {connect} from 'react-redux';
 import {SelectBox, MultiSelectBox} from '@neos-project/react-ui-components';
 import {selectors} from '@neos-project/neos-ui-redux-store';
 import {neos} from '@neos-project/neos-ui-decorators';
-import {shouldDisplaySearchBox, searchOptions, processSelectBoxOptions} from './SelectBoxHelpers';
+import {shouldDisplaySearchBox, searchOptions, processSelectBoxOptions} from './selectBoxHelpers';
 import {createSelectBoxValueStringFromPossiblyStrangeNodePropertyValue} from './createSelectBoxValueStringFromPossiblyStrangeNodePropertyValue';
 import PreviewOption from '../../Library/PreviewOption';
 
@@ -130,7 +130,10 @@ export default class DataSourceBasedSelectBoxEditor extends PureComponent {
         const {commit, i18nRegistry, className} = this.props;
         const options = Object.assign({}, this.constructor.defaultOptions, this.props.options);
 
-        const processedSelectBoxOptions = processSelectBoxOptions(i18nRegistry, this.state.selectBoxOptions);
+        const processedValue = options.multiple ? this.valueForMultiSelect : this.valueForSingleSelect;
+
+        // we have to wait till the options are loaded as otherwise everything will be shown as "invalid" and is a mismatch
+        const processedSelectBoxOptions = this.state.isLoading ? [] : processSelectBoxOptions(i18nRegistry, this.state.selectBoxOptions, processedValue);
 
         // Placeholder text must be unescaped in case html entities were used
         const placeholder = options && options.placeholder && i18nRegistry.translate(unescape(options.placeholder));
@@ -140,7 +143,7 @@ export default class DataSourceBasedSelectBoxEditor extends PureComponent {
             return (<MultiSelectBox
                 className={className}
                 options={processedSelectBoxOptions}
-                values={this.valueForMultiSelect}
+                values={processedValue}
                 onValuesChange={commit}
                 loadingLabel={loadingLabel}
                 ListPreviewElement={PreviewOption}
@@ -161,7 +164,7 @@ export default class DataSourceBasedSelectBoxEditor extends PureComponent {
         return (<SelectBox
             className={className}
             options={this.state.searchTerm ? searchOptions(this.state.searchTerm, processedSelectBoxOptions) : processedSelectBoxOptions}
-            value={this.valueForSingleSelect}
+            value={processedValue}
             onValueChange={commit}
             loadingLabel={loadingLabel}
             ListPreviewElement={PreviewOption}

packages/neos-ui-editors/src/Editors/SelectBox/SimpleSelectBoxEditor.js

@@ -2,7 +2,7 @@ import React, {PureComponent} from 'react';
 import PropTypes from 'prop-types';
 import {SelectBox, MultiSelectBox} from '@neos-project/react-ui-components';
 import {neos} from '@neos-project/neos-ui-decorators';
-import {shouldDisplaySearchBox, searchOptions, processSelectBoxOptions} from './SelectBoxHelpers';
+import {shouldDisplaySearchBox, searchOptions, processSelectBoxOptions} from './selectBoxHelpers';
 
 @neos(globalRegistry => ({
     i18nRegistry: globalRegistry.get('i18n')
@@ -48,10 +48,10 @@ export default class SimpleSelectBoxEditor extends PureComponent {
     };
 
     render() {
-        const {commit, value, i18nRegistry, className} = this.props;
+        const {commit, i18nRegistry, className, value} = this.props;
         const options = Object.assign({}, this.constructor.defaultOptions, this.props.options);
 
-        const processedSelectBoxOptions = processSelectBoxOptions(i18nRegistry, options.values);
+        const processedSelectBoxOptions = processSelectBoxOptions(i18nRegistry, options.values, value);
 
         const allowEmpty = options.allowEmpty || Object.prototype.hasOwnProperty.call(options.values, '');
 

packages/neos-ui-editors/src/Editors/SelectBox/selectBoxHelpers.spec.ts

@@ -0,0 +1,127 @@
+import {processSelectBoxOptions} from './selectBoxHelpers';
+import {I18nRegistry} from '@neos-project/neos-ts-interfaces';
+
+const fakeI18NRegistry: I18nRegistry = {
+    translate: (id) => id ?? ''
+};
+
+describe('processSelectBoxOptions', () => {
+    it('transforms an associative array with labels to list of objects', () => {
+        const processOptions = processSelectBoxOptions(fakeI18NRegistry, {
+            'key1': {label: 'Key 1'},
+            'key2': {label: 'Key 2', icon: 'foo', disabled: true}
+        }, null);
+
+        expect(processOptions).toEqual([{value: 'key1', label: 'Key 1'}, {value: 'key2', label: 'Key 2', icon: 'foo', disabled: true}]);
+    });
+
+    it('keeps valid shape of list of objects intact', () => {
+        const options = [{value: 'key1', label: 'Key 1'}, {value: 'key2', label: 'Key 2', icon: 'foo', disabled: true}];
+        const processOptions = processSelectBoxOptions(fakeI18NRegistry, options, null);
+
+        expect(processOptions).toEqual(options);
+    });
+
+    it('overrules the array key with the explicit value', () => {
+        const processOptions = processSelectBoxOptions(fakeI18NRegistry, {
+            'key1': {label: 'Key 1'},
+            // @ts-expect-error we declare the typescript types to what we want, but cant influence user input
+            'key2': {label: 'Key 2', value: 'key2-overrule'}
+        }, null);
+
+        expect(processOptions).toEqual([{value: 'key1', label: 'Key 1'}, {value: 'key2-overrule', label: 'Key 2'}]);
+    });
+
+    it('uses numeric string array key for list of objects', () => {
+        const processOptions = processSelectBoxOptions(fakeI18NRegistry, [
+            {value: 'key1', label: 'Key 1'},
+            {label: 'Key 2'}
+        ] as any, null);
+
+        expect(processOptions).toEqual([{value: 'key1', label: 'Key 1'}, {value: '1', label: 'Key 2'}]);
+    });
+
+    it('omits entries that are invalid and empty', () => {
+        let processOptions = processSelectBoxOptions(fakeI18NRegistry, {
+            'key1': {label: 'Key 1'},
+            // @ts-expect-error we declare the typescript types to what we want, but cant influence user input
+            'key2': null
+        }, null);
+
+        expect(processOptions).toEqual([{value: 'key1', label: 'Key 1'}]);
+
+        processOptions = processSelectBoxOptions(fakeI18NRegistry, {
+            'key1': {label: 'Key 1'},
+            // @ts-expect-error we declare the typescript types to what we want, but cant influence user input
+            'key2': {}
+        }, null);
+
+        expect(processOptions).toEqual([{value: 'key1', label: 'Key 1'}]);
+
+        processOptions = processSelectBoxOptions(fakeI18NRegistry, [
+            {value: 'key1', label: 'Key 1'},
+            {value: 'key2'}
+        ] as any, null);
+
+        expect(processOptions).toEqual([{value: 'key1', label: 'Key 1'}]);
+
+        processOptions = processSelectBoxOptions(fakeI18NRegistry, [
+            {value: 'key1', label: 'Key 1'},
+            {}
+        ] as any, null);
+
+        expect(processOptions).toEqual([{value: 'key1', label: 'Key 1'}]);
+    });
+
+    it('creates missing option for unmatched string value', () => {
+        const processOptions = processSelectBoxOptions(fakeI18NRegistry, {
+            'key1': {label: 'Key 1'}
+        }, 'oldValue');
+
+        expect(processOptions).toEqual([{value: 'key1', label: 'Key 1'}, {value: 'oldValue', label: 'Neos.Neos.Ui:Main:invalidValue: "oldValue"', icon: 'exclamation-triangle'}]);
+    });
+
+    it('creates missing options for unmatched additional array value', () => {
+        const processOptions = processSelectBoxOptions(fakeI18NRegistry, {
+            'key1': {label: 'Key 1'}
+        }, ['oldValue', 'key1']);
+
+        expect(processOptions).toEqual([{value: 'key1', label: 'Key 1'}, {value: 'oldValue', label: 'Neos.Neos.Ui:Main:invalidValue: "oldValue"', icon: 'exclamation-triangle'}]);
+    });
+
+    it('creates missing options for unmatched additional multiple array values', () => {
+        const processOptions = processSelectBoxOptions(
+            fakeI18NRegistry,
+            [{value: 'key1', label: 'Key 1'}, {value: 'key2', label: 'Key 2'}, {value: 'key3', label: 'Key 3'}],
+            ['oldValue', 'key1', 'oldValue2']
+        );
+
+        expect(processOptions).toEqual([
+            {value: 'key1', label: 'Key 1'},
+            {value: 'key2', label: 'Key 2'},
+            {value: 'key3', label: 'Key 3'},
+            {value: 'oldValue', label: 'Neos.Neos.Ui:Main:invalidValue: "oldValue"', icon: 'exclamation-triangle'},
+            {value: 'oldValue2', label: 'Neos.Neos.Ui:Main:invalidValue: "oldValue2"', icon: 'exclamation-triangle'}
+        ]);
+    });
+
+    it('ignored current value being empty and dont create missing option', () => {
+        let processOptions = processSelectBoxOptions(fakeI18NRegistry, {
+            'key1': {label: 'Key 1'}
+        }, null);
+
+        expect(processOptions).toEqual([{value: 'key1', label: 'Key 1'}]);
+
+        processOptions = processSelectBoxOptions(fakeI18NRegistry, {
+            'key1': {label: 'Key 1'}
+        }, undefined);
+
+        expect(processOptions).toEqual([{value: 'key1', label: 'Key 1'}]);
+
+        processOptions = processSelectBoxOptions(fakeI18NRegistry, {
+            'key1': {label: 'Key 1'}
+        }, '');
+
+        expect(processOptions).toEqual([{value: 'key1', label: 'Key 1'}]);
+    });
+});

packages/neos-ui-editors/src/Editors/SelectBox/selectBoxHelpers.ts

@@ -0,0 +1,51 @@
+import {I18nRegistry} from '@neos-project/neos-ts-interfaces';
+import {isNil} from '@neos-project/utils-helpers';
+
+type RawSelectBoxOptions = {value: string, icon?: string; disabled?: boolean; label: string;}[]|{[key: string]: {icon?: string; disabled?: boolean; label: string;}};
+
+type SelectBoxOptions = {value: string, icon?: string; disabled?: boolean; label: string;}[];
+
+export const shouldDisplaySearchBox = (options: any, processedSelectBoxOptions: SelectBoxOptions) => options.minimumResultsForSearch >= 0 && processedSelectBoxOptions.length >= options.minimumResultsForSearch;
+
+// Currently, we're doing an extremely simple lowercase substring matching; of course this could be improved a lot!
+export const searchOptions = (searchTerm: string, processedSelectBoxOptions: SelectBoxOptions) =>
+    processedSelectBoxOptions.filter(option => option.label && option.label.toLowerCase().indexOf(searchTerm.toLowerCase()) !== -1);
+
+export const processSelectBoxOptions = (i18nRegistry: I18nRegistry, selectBoxOptions: RawSelectBoxOptions, currentValue: unknown): SelectBoxOptions => {
+    const validValues: Record<string, true> = {};
+    const processedSelectBoxOptions = [];
+    for (const [key, selectBoxOption] of Object.entries(selectBoxOptions)) {
+        if (!selectBoxOption || !selectBoxOption.label) {
+            continue;
+        }
+
+        const processedSelectBoxOption = {
+            value: key,
+            ...selectBoxOption, // a value in here overrules value based on the key above.
+            label: i18nRegistry.translate(selectBoxOption.label)
+        };
+
+        validValues[processedSelectBoxOption.value] = true;
+        processedSelectBoxOptions.push(processedSelectBoxOption);
+    }
+
+    const valueIsEmpty = isNil(currentValue) || currentValue === '';
+    if (valueIsEmpty) {
+        return processedSelectBoxOptions;
+    }
+
+    for (const singleValue of Array.isArray(currentValue) ? currentValue : [currentValue]) {
+        if (singleValue in validValues) {
+            continue;
+        }
+
+        // Mismatch detected. Thus we add an option to the schema so the value is displayable: https://github.com/neos/neos-ui/issues/3520
+        processedSelectBoxOptions.push({
+            value: singleValue,
+            label: `${i18nRegistry.translate('Neos.Neos.Ui:Main:invalidValue')}: "${singleValue}"`,
+            icon: 'exclamation-triangle'
+        });
+    }
+
+    return processedSelectBoxOptions;
+}

packages/neos-ui/src/Containers/LeftSideBar/NodeTreeSearchBar/NodeTreeFilter/index.js

@@ -5,7 +5,7 @@ import {$get} from 'plow-js';
 import {neos} from '@neos-project/neos-ui-decorators';
 import {SelectBox} from '@neos-project/react-ui-components';
 
-import {searchOptions} from '@neos-project/neos-ui-editors/src/Editors/SelectBox/SelectBoxHelpers.js';
+import {searchOptions} from '@neos-project/neos-ui-editors/src/Editors/SelectBox/selectBoxHelpers.js';
 
 import style from './style.module.css';
 

packages/react-ui-components/src/MultiSelectBox_ListPreviewSortable/multiSelectBox_ListPreviewSortable.js

@@ -61,35 +61,33 @@ export default class MultiSelectBox_ListPreviewSortable extends PureComponent {
             options,
             optionValueAccessor
         } = this.props;
-
         const {draggableValues} = this.state;
+        const {DraggableListPreviewElement} = this;
 
         // Sorted options by draggable value ordering
         const draggableOptions = draggableValues.map(value =>
             options.find(option => optionValueAccessor(option) === value)
         ).filter(Boolean);
 
-        return draggableOptions.map(this.renderOption);
-    }
-
-    renderOption = (option, index) => {
-        const {
-            optionValueAccessor
-        } = this.props;
-
-        const {DraggableListPreviewElement} = this;
-
-        return (
-            <DraggableListPreviewElement
-                {...this.props}
-                key={optionValueAccessor(option)}
-                index={index}
-                option={option}
-                onMoveSelectedValue={this.handleMoveSelectedValue}
-                onSelectedValueWasMoved={this.handleSelectedValueWasMoved}
-                onRemoveItem={this.handleRemoveItem}
+        return draggableOptions.map((option, index) => {
+            if (!option) {
+                // if the value doesn't match an option we ignore it.
+                // though we must be careful that the correct `index` is preserved for succeeding entries.
+                // https://github.com/neos/neos-ui/issues/3520#issuecomment-2185969334
+                return '';
+            }
+            return (
+                <DraggableListPreviewElement
+                    {...this.props}
+                    key={optionValueAccessor(option)}
+                    index={index}
+                    option={option}
+                    onMoveSelectedValue={this.handleMoveSelectedValue}
+                    onSelectedValueWasMoved={this.handleSelectedValueWasMoved}
+                    onRemoveItem={this.handleRemoveItem}
                 />
-        );
+            );
+        });
     }
 
     handleMoveSelectedValue = (dragIndex, hoverIndex) => {

packages/react-ui-components/src/SelectBox/__snapshots__/selectBox.spec.js.snap

@@ -27,7 +27,6 @@ exports[`<SelectBox/> should render correctly. 1`] = `
       scrollable={true}
       searchBoxLeftToTypeLabel="searchBoxLeftToTypeLabel"
       showDropDownToggle={true}
-      showResetButton={false}
       theme={
         Object {
           "selectBoxHeader": "selectBoxHeaderClassName",

packages/react-ui-components/src/SelectBox/selectBox.js

@@ -271,12 +271,11 @@ export default class SelectBox extends PureComponent {
         // Compare selected value less strictly: allow loose comparision and deep equality of objects
         const selectedOption = options.find(option => optionValueAccessor(option) == value || isEqual(optionValueAccessor(option), value)); // eslint-disable-line eqeqeq
 
+        /* eslint-disable no-eq-null, eqeqeq */ // to check for null or undefined, we cannot use the isNil helper as it's not published to npm
+        const valueIsEmpty = value == null || value === '';
         if (
             displaySearchBox && (
-                // check for null or undefined
-                /* eslint-disable no-eq-null, eqeqeq */
-                value == null ||
-                value === '' ||
+                valueIsEmpty ||
                 this.state.isExpanded ||
                 plainInputMode
             )
@@ -292,7 +291,7 @@ export default class SelectBox extends PureComponent {
             );
         }
 
-        const showResetButton = Boolean(allowEmpty && !displayLoadingIndicator && value);
+        const showResetButton = allowEmpty && !displayLoadingIndicator && !valueIsEmpty;
 
         return (
             <SelectBox_Header