Skip to content

Commit

Permalink
fix: ensure values not in options clear
Browse files Browse the repository at this point in the history
Closes #817
  • Loading branch information
Skaiir committed Nov 23, 2023
1 parent c031664 commit 8435f7a
Show file tree
Hide file tree
Showing 9 changed files with 358 additions and 3 deletions.
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { useContext, useEffect, useRef } from 'preact/hooks';
import { useContext, useRef } from 'preact/hooks';
import useOptionsAsync, { LOAD_STATES } from '../../hooks/useOptionsAsync';
import useCleanupMultiSelectValues from '../../hooks/useCleanupMultiSelectValues';
import classNames from 'classnames';
import { FormContext } from '../../context';

Expand Down Expand Up @@ -80,6 +81,14 @@ export default function Checklist(props) {
options
} = useOptionsAsync(field);

useCleanupMultiSelectValues({
field,
loadState,
options,
values,
onChange: props.onChange
});

const { formId } = useContext(FormContext);
const errorMessageId = errors.length === 0 ? undefined : `${prefixId(id, formId)}-error-message`;

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { useContext, useRef } from 'preact/hooks';
import useOptionsAsync, { LOAD_STATES } from '../../hooks/useOptionsAsync';
import useCleanupSingleSelectValue from '../../hooks/useCleanupSingleSelectValue';
import classNames from 'classnames';
import { FormContext } from '../../context';

Expand Down Expand Up @@ -69,6 +70,14 @@ export default function Radio(props) {
options
} = useOptionsAsync(field);

useCleanupSingleSelectValue({
field,
loadState,
options,
value,
onChange: props.onChange
});

const { formId } = useContext(FormContext);
const errorMessageId = errors.length === 0 ? undefined : `${prefixId(id, formId)}-error-message`;

Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import { useContext, useMemo, useRef, useState } from 'preact/hooks';

import useOptionsAsync, { LOAD_STATES } from '../../hooks/useOptionsAsync';
import { useService } from '../../hooks';
import useOptionsAsync, { LOAD_STATES } from '../../hooks/useOptionsAsync';
import useCleanupMultiSelectValues from '../../hooks/useCleanupMultiSelectValues';

import { FormContext } from '../../context';
import classNames from 'classnames';
Expand Down Expand Up @@ -59,6 +60,14 @@ export default function Taglist(props) {
options
} = useOptionsAsync(field);

useCleanupMultiSelectValues({
field,
loadState,
options,
values,
onChange: props.onChange
});

// We cache a map of option values to their index so that we don't need to search the whole options array every time to correlate the label
const valueToOptionMap = useMemo(() => Object.assign({}, ...options.map((o, x) => ({ [o.value]: options[x] }))), [ options ]);

Expand Down Expand Up @@ -189,7 +198,7 @@ export default function Taglist(props) {
return (
<div class={ classNames('fjs-taglist-tag', { 'fjs-disabled': disabled, 'fjs-readonly': readonly }) } onMouseDown={ (e) => e.preventDefault() }>
<span class="fjs-taglist-tag-label">
{valueToOptionMap[v] ? valueToOptionMap[v].label : `unexpected value{${v}}`}
{ valueToOptionMap[v] ? valueToOptionMap[v].label : undefined }
</span>
{ (!disabled && !readonly) && <button
type="button"
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { useCallback, useContext, useEffect, useMemo, useRef, useState } from 'preact/hooks';
import useOptionsAsync, { LOAD_STATES } from '../../../hooks/useOptionsAsync';
import { useService } from '../../../hooks';
import useCleanupSingleSelectValue from '../../../hooks/useCleanupSingleSelectValue';

import { FormContext } from '../../../context';

Expand Down Expand Up @@ -41,6 +42,14 @@ export default function SearchableSelect(props) {
options
} = useOptionsAsync(field);

useCleanupSingleSelectValue({
field,
loadState,
options,
value,
onChange: props.onChange
});

// We cache a map of option values to their index so that we don't need to search the whole options array every time to correlate the label
const valueToOptionMap = useMemo(() => Object.assign({}, ...options.map((o, x) => ({ [o.value]: options[x] }))), [ options ]);

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { useCallback, useContext, useMemo, useRef, useState } from 'preact/hooks';
import useOptionsAsync, { LOAD_STATES } from '../../../hooks/useOptionsAsync';
import useCleanupSingleSelectValue from '../../../hooks/useCleanupSingleSelectValue';

import { FormContext } from '../../../context';

Expand Down Expand Up @@ -38,6 +39,14 @@ export default function SimpleSelect(props) {
options
} = useOptionsAsync(field);

useCleanupSingleSelectValue({
field,
loadState,
options,
value,
onChange: props.onChange
});

// We cache a map of option values to their index so that we don't need to search the whole options array every time to correlate the label
const valueToOptionMap = useMemo(() => Object.assign({}, ...options.map((o, x) => ({ [o.value]: options[x] }))), [ options ]);

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
import { useEffect } from 'preact/hooks';
import { LOAD_STATES } from './useOptionsAsync';

export default function(props) {

const {
field,
options,
loadState,
onChange,
values
} = props;

// Ensures that the values are always a subset of the possible options
useEffect(() => {

if (loadState !== LOAD_STATES.LOADED) {
return;
}

const hasValuesNotInOptions = values.some(v => !options.map(o => o.value).includes(v));

if (hasValuesNotInOptions) {
onChange({
field,
value: values.filter(v => options.map(o => o.value).includes(v))
});
}

}, [ field, options, onChange, JSON.stringify(values), loadState ]);

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
import { useEffect } from 'preact/hooks';
import { LOAD_STATES } from './useOptionsAsync';

export default function(props) {

const {
field,
options,
loadState,
onChange,
value
} = props;

// Ensures that the value is always one of the possible options
useEffect(() => {

if (loadState !== LOAD_STATES.LOADED) {
return;
}

const hasValueNotInOptions = value && !options.map(o => o.value).includes(value);

if (hasValueNotInOptions) {
onChange({
field,
value: null
});
}

}, [ field, options, onChange, value, loadState ]);

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,123 @@
import {
render,
} from '@testing-library/preact/pure';

import useCleanupMultiSelectValues from '../../../../src/render/hooks/useCleanupMultiSelectValues';

const spy = sinon.spy;
let root;

describe('useCleanupMultiSelectValue', function() {

beforeEach(() => root = document.createElement('div'));

afterEach(() => root.remove());

it('should fire onChange when any value is no longer in the options', function() {

// given
const onChangeSpy = spy();
const values = [ 'camunda-platform', 'camunda-cloud' ];

const { rerender } = render(
<TestComponent onChangeSpy={ onChangeSpy } options={ baseOptions } values={ values } />, {
container: root
});

// when
rerender(
<TestComponent onChangeSpy={ onChangeSpy } options={ noPlatformOptions } values={ values } />, {
container: root
}
);

// then
expect(onChangeSpy).to.have.been.calledOnce;
expect(onChangeSpy).to.have.been.calledWith({
field: 'foo',
value: [ 'camunda-cloud' ]
});

});


it('should not fire onChange when all values are still in the options', function() {

// given
const onChangeSpy = spy();
const values = [ 'camunda-platform', 'camunda-cloud' ];

const { rerender } = render(
<TestComponent onChangeSpy={ onChangeSpy } options={ baseOptions } values={ values } />, {
container: root
});

// when
rerender(
<TestComponent onChangeSpy={ onChangeSpy } options={ baseOptions } values={ values } />, {
container: root
}
);

// then
expect(onChangeSpy).to.not.have.been.called;

});


it('should not reinstate the removed values when the options change back', function() {

// given
const onChangeSpy = spy();
const values = [ 'camunda-platform', 'camunda-cloud' ];

const { rerender } = render(
<TestComponent onChangeSpy={ onChangeSpy } options={ baseOptions } values={ values } />, {
container: root
});

// when
rerender(
<TestComponent onChangeSpy={ onChangeSpy } options={ noPlatformOptions } values={ values } />, {
container: root
}
);

rerender(
<TestComponent onChangeSpy={ onChangeSpy } options={ baseOptions } values={ values } />, {
container: root
}
);

// then
expect(onChangeSpy).to.have.been.calledOnce;
expect(onChangeSpy).to.have.been.calledWith({
field: 'foo',
value: [ 'camunda-cloud' ]
});

});

});

const baseOptions = [
{ value: 'camunda-platform', label: 'Camunda Platform' },
{ value: 'camunda-cloud', label: 'Camunda Cloud' }
];

const noPlatformOptions = [
{ value: 'camunda-cloud', label: 'Camunda Cloud' }
];

function TestComponent({ onChangeSpy, options, values }) {

useCleanupMultiSelectValues({
field: 'foo',
loadState: 'loaded',
onChange: onChangeSpy,
options,
values
});

return <></>;
}
Loading

0 comments on commit 8435f7a

Please sign in to comment.