diff --git a/e2e/visual/carbon-styles.spec.js-snapshots/carbon-styles-1-chromium-linux.png b/e2e/visual/carbon-styles.spec.js-snapshots/carbon-styles-1-chromium-linux.png index 526183116..aac62955c 100644 Binary files a/e2e/visual/carbon-styles.spec.js-snapshots/carbon-styles-1-chromium-linux.png and b/e2e/visual/carbon-styles.spec.js-snapshots/carbon-styles-1-chromium-linux.png differ diff --git a/packages/form-js-editor/src/features/render-injection/slot-fill/SlotFillRoot.js b/packages/form-js-editor/src/features/render-injection/slot-fill/SlotFillRoot.js index 1ed763d52..72be9c78b 100644 --- a/packages/form-js-editor/src/features/render-injection/slot-fill/SlotFillRoot.js +++ b/packages/form-js-editor/src/features/render-injection/slot-fill/SlotFillRoot.js @@ -2,19 +2,26 @@ import FillContext from './FillContext'; import SlotContext from './SlotContext'; import { useMemo, useState } from 'preact/hooks'; +const noop = () => {}; + export default (props) => { const [ fills, setFills ] = useState([]); + const { + onSetFill = noop, + onRemoveFill = noop + } = props; + const fillContext = useMemo(() => ({ addFill: (fill) => { setFills((fills) => [ ...fills.filter((f) => f.id !== fill.id), fill ]); - props.onSetFill && props.onSetFill(fill); + onSetFill(fill); }, removeFill: (id) => { setFills((fills) => fills.filter((f) => f.id !== id)); - props.onRemoveFill && props.onRemoveFill(id); + onRemoveFill(id); } - }), []); + }), [ onRemoveFill, onSetFill ]); const slotContext = useMemo(() => ({ fills }), [ fills ]); diff --git a/packages/form-js-editor/src/render/components/FormEditor.js b/packages/form-js-editor/src/render/components/FormEditor.js index 77ca3b8bc..b253e0514 100644 --- a/packages/form-js-editor/src/render/components/FormEditor.js +++ b/packages/form-js-editor/src/render/components/FormEditor.js @@ -110,24 +110,25 @@ function Element(props) { const [ hovered, setHovered ] = useState(false); - function scrollIntoView({ selection }) { - if (!selection || selection.id !== id || !ref.current) { - return; - } + useEffect(() => { - const elementBounds = ref.current.getBoundingClientRect(), - containerBounds = formEditor._container.getBoundingClientRect(); + function scrollIntoView({ selection }) { + if (!selection || selection.id !== id || !ref.current) { + return; + } - if (elementBounds.top < 0 || elementBounds.top > containerBounds.bottom) { - ref.current.scrollIntoView(); + const elementBounds = ref.current.getBoundingClientRect(), + containerBounds = formEditor._container.getBoundingClientRect(); + + if (elementBounds.top < 0 || elementBounds.top > containerBounds.bottom) { + ref.current.scrollIntoView(); + } } - } - useEffect(() => { eventBus.on('selection.changed', scrollIntoView); return () => eventBus.off('selection.changed', scrollIntoView); - }, [ id ]); + }, [ eventBus, formEditor._container, id ]); useLayoutEffect(() => { if (selection.isSelected(field)) { @@ -320,7 +321,7 @@ function Column(props) { ); } -export default function FormEditor(props) { +export default function FormEditor() { const dragging = useService('dragging'), eventBus = useService('eventBus'), formEditor = useService('formEditor'), @@ -339,6 +340,8 @@ export default function FormEditor(props) { const [ , setSelection ] = useState(schema); + const [ hasInitialized, setHasInitialized ] = useState(false); + useEffect(() => { function handleSelectionChanged(event) { setSelection(event.selection || schema); @@ -346,12 +349,14 @@ export default function FormEditor(props) { eventBus.on('selection.changed', handleSelectionChanged); - setSelection(selection.get() || schema); - return () => { eventBus.off('selection.changed', handleSelectionChanged); }; - }, [ schema, selection ]); + }, [ eventBus, schema ]); + + useEffect(() => { + setSelection(selection.get() || schema); + }, [ selection, schema ]); const [ drake, setDrake ] = useState(null); @@ -424,11 +429,17 @@ export default function FormEditor(props) { // fire event after render to notify interested parties useEffect(() => { + + if (hasInitialized) { + return; + } + + setHasInitialized(true); eventBus.fire('rendered'); // keep deprecated event to ensure backward compatibility eventBus.fire('formEditor.rendered'); - }, []); + }, [ eventBus, hasInitialized ]); const formRenderContext = useMemo(() => ({ Children, @@ -523,69 +534,69 @@ function CreatePreview(props) { const formFields = useService('formFields'); - function handleCloned(clone, original, type) { - - const fieldType = clone.dataset.fieldType; + useEffect(() => { + if (!drake) { + return; + } - // (1) field preview - if (fieldType) { + function handleCloned(clone, original, type) { - const paletteEntry = findPaletteEntry(fieldType, formFields); + const fieldType = clone.dataset.fieldType; - if (!paletteEntry) { - return; - } + // (1) field preview + if (fieldType) { - const { label } = paletteEntry; + const paletteEntry = findPaletteEntry(fieldType, formFields); - const Icon = getPaletteIcon(paletteEntry); + if (!paletteEntry) { + return; + } - clone.innerHTML = ''; + const { label } = paletteEntry; - clone.class = 'gu-mirror'; - clone.classList.add('fjs-field-preview-container'); + const Icon = getPaletteIcon(paletteEntry); - if (original.classList.contains('fjs-palette-field')) { + clone.innerHTML = ''; - // default to auto columns when creating from palette - clone.classList.add('cds--col'); - } + clone.class = 'gu-mirror'; + clone.classList.add('fjs-field-preview-container'); - // todo(pinussilvestrus): dragula, how to mitigate cursor position - // https://github.com/bevacqua/dragula/issues/285 - render( - , - clone - ); - } else { - - // (2) row preview - - // remove elements from copy (context pad, row dragger, ...) - [ - 'fjs-context-pad', - 'fjs-row-dragger', - 'fjs-debug-columns' - ].forEach(cls => { - const cloneNode = clone.querySelectorAll('.' + cls); - cloneNode.length && cloneNode.forEach(e => e.remove()); - }); + if (original.classList.contains('fjs-palette-field')) { - // mirror grid - clone.classList.add('cds--grid'); - clone.classList.add('cds--grid--condensed'); - } - } + // default to auto columns when creating from palette + clone.classList.add('cds--col'); + } - useEffect(() => { - if (!drake) { - return; + // todo(pinussilvestrus): dragula, how to mitigate cursor position + // https://github.com/bevacqua/dragula/issues/285 + render( + , + clone + ); + } else { + + // (2) row preview + + // remove elements from copy (context pad, row dragger, ...) + [ + 'fjs-context-pad', + 'fjs-row-dragger', + 'fjs-debug-columns' + ].forEach(cls => { + const cloneNode = clone.querySelectorAll('.' + cls); + cloneNode.length && cloneNode.forEach(e => e.remove()); + }); + + // mirror grid + clone.classList.add('cds--grid'); + clone.classList.add('cds--grid--condensed'); + } } drake.on('cloned', handleCloned); return () => drake.off('cloned', handleCloned); - }, [ drake ]); + }, [ drake, formFields ]); return null; } diff --git a/packages/form-js-editor/src/render/hooks/useDebounce.js b/packages/form-js-editor/src/render/hooks/useDebounce.js index 1a51b654f..5c4ac076b 100644 --- a/packages/form-js-editor/src/render/hooks/useDebounce.js +++ b/packages/form-js-editor/src/render/hooks/useDebounce.js @@ -5,14 +5,16 @@ import { import useService from './useService'; - -export default function useDebounce(fn, dependencies = []) { +/** + * @param {Function} fn - function to debounce + */ +export default function useDebounce(fn) { const debounce = useService('debounce'); const callback = useMemo(() => { return debounce(fn); - }, dependencies); + }, [ debounce, fn ]); // cleanup async side-effect if callback #flush is provided. useEffect(() => { diff --git a/packages/form-js-editor/src/render/hooks/usePrevious.js b/packages/form-js-editor/src/render/hooks/usePrevious.js index 99bc24913..21a3bfbf1 100644 --- a/packages/form-js-editor/src/render/hooks/usePrevious.js +++ b/packages/form-js-editor/src/render/hooks/usePrevious.js @@ -4,10 +4,10 @@ import { } from 'preact/hooks'; -export default function usePrevious(value) { - const ref = useRef(); +export default function usePrevious(value, defaultValue = null) { + const ref = useRef(defaultValue); - useEffect(() => ref.current = value); + useEffect(() => ref.current = value, [ value ]); return ref.current; } \ No newline at end of file diff --git a/packages/form-js-playground/src/components/PlaygroundRoot.js b/packages/form-js-playground/src/components/PlaygroundRoot.js index 4b242ff31..94e1c449c 100644 --- a/packages/form-js-playground/src/components/PlaygroundRoot.js +++ b/packages/form-js-playground/src/components/PlaygroundRoot.js @@ -33,7 +33,9 @@ export function PlaygroundRoot(props) { editorProperties = {}, viewerAdditionalModules = [], editorAdditionalModules = [], - propertiesPanel: propertiesPanelConfig = {} + propertiesPanel: propertiesPanelConfig = {}, + onInit: onPlaygroundInit, + onStateChanged } = props; const { @@ -66,7 +68,7 @@ export function PlaygroundRoot(props) { // pipe to playground API useEffect(() => { - props.onInit({ + onPlaygroundInit({ attachDataContainer: (node) => dataEditorRef.current.attachTo(node), attachEditorContainer: (node) => formEditorRef.current.attachTo(node), attachFormContainer: (node) => formRef.current.attachTo(node), @@ -82,7 +84,7 @@ export function PlaygroundRoot(props) { setSchema: setInitialSchema, saveSchema: () => formEditorRef.current.saveSchema() }); - }); + }, [ onPlaygroundInit ]); useEffect(() => { setInitialSchema(props.schema || {}); @@ -230,11 +232,11 @@ export function PlaygroundRoot(props) { }, [ resultData ]); useEffect(() => { - props.onStateChanged({ + onStateChanged && onStateChanged({ schema, data }); - }, [ schema, data ]); + }, [ onStateChanged, schema, data ]); const handleDownload = useCallback(() => { diff --git a/packages/form-js-viewer/src/features/repeatRender/RepeatRenderManager.js b/packages/form-js-viewer/src/features/repeatRender/RepeatRenderManager.js index ddb3fef5b..5f2454f1e 100644 --- a/packages/form-js-viewer/src/features/repeatRender/RepeatRenderManager.js +++ b/packages/form-js-viewer/src/features/repeatRender/RepeatRenderManager.js @@ -80,10 +80,10 @@ export default class RepeatRenderManager { return ( <> {displayValues.map((value, index) => { - const elementProps = { + const elementProps = useMemo(() => ({ ...restProps, - indexes: { ...(indexes || {}), [ repeaterField.id ]: index }, - }; + indexes: { ...(indexes || {}), [ repeaterField.id ]: index } + }), [ index ]); const localExpressionContextInfo = useMemo(() => ({ data: parentExpressionContextInfo.data, diff --git a/packages/form-js-viewer/src/render/Renderer.js b/packages/form-js-viewer/src/render/Renderer.js index e3fe20781..78255d268 100644 --- a/packages/form-js-viewer/src/render/Renderer.js +++ b/packages/form-js-viewer/src/render/Renderer.js @@ -34,7 +34,7 @@ export default function Renderer(config, eventBus, form, injector) { setState(newState); }); - const onChange = useCallback((update) => form._update(update), [ form ]); + const onChange = useCallback((update) => form._update(update), []); const { properties } = state; @@ -44,9 +44,9 @@ export default function Renderer(config, eventBus, form, injector) { if (!readOnly) { form.submit(); } - }, [ form, readOnly ]); + }, [ readOnly ]); - const onReset = useCallback(() => form.reset(), [ form ]); + const onReset = useCallback(() => form.reset(), []); const { schema } = state; diff --git a/packages/form-js-viewer/src/render/components/FormField.js b/packages/form-js-viewer/src/render/components/FormField.js index 971f1c7f8..ebd5b2bc1 100644 --- a/packages/form-js-viewer/src/render/components/FormField.js +++ b/packages/form-js-viewer/src/render/components/FormField.js @@ -77,7 +77,7 @@ export default function FormField(props) { if (viewerCommands && initialValue) { viewerCommands.updateFieldValidation(field, initialValue, indexes); } - }, [ viewerCommands, field, initialValue, JSON.stringify(indexes) ]); + }, [ viewerCommands, field, initialValue, indexes ]); const hidden = useCondition(field.conditional && field.conditional.hide || null); diff --git a/packages/form-js-viewer/src/render/components/form-fields/Taglist.js b/packages/form-js-viewer/src/render/components/form-fields/Taglist.js index 790c19c12..af7e0d5cc 100644 --- a/packages/form-js-viewer/src/render/components/form-fields/Taglist.js +++ b/packages/form-js-viewer/src/render/components/form-fields/Taglist.js @@ -1,6 +1,6 @@ import { useMemo, useRef, useState } from 'preact/hooks'; -import { useService } from '../../hooks'; +import { useDeepCompareState, useService } from '../../hooks'; import useOptionsAsync, { LOAD_STATES } from '../../hooks/useOptionsAsync'; import useCleanupMultiSelectValues from '../../hooks/useCleanupMultiSelectValues'; import { useGetLabelCorrelation } from '../../hooks/useGetLabelCorrelation'; @@ -35,7 +35,7 @@ export default function Taglist(props) { onBlur, field, readonly, - value : values = [] + value } = props; const { @@ -58,6 +58,9 @@ export default function Taglist(props) { options } = useOptionsAsync(field); + // ensures we render based on array content instead of reference + const values = useDeepCompareState(value || [], []); + useCleanupMultiSelectValues({ field, loadState, @@ -70,7 +73,6 @@ export default function Taglist(props) { const hasOptionsLeft = useMemo(() => options.length > values.length, [ options.length, values.length ]); - // Usage of stringify is necessary here because we want this effect to only trigger when there is a value change to the array const filteredOptions = useMemo(() => { if (loadState !== LOAD_STATES.LOADED) { return []; @@ -82,7 +84,7 @@ export default function Taglist(props) { }; return options.filter(isValidFilteredOption); - }, [ filter, options, JSON.stringify(values), loadState ]); + }, [ filter, options, values, loadState ]); const selectValue = (value) => { diff --git a/packages/form-js-viewer/src/render/components/form-fields/Textarea.js b/packages/form-js-viewer/src/render/components/form-fields/Textarea.js index 3b2fdf5a0..8e0504e5b 100644 --- a/packages/form-js-viewer/src/render/components/form-fields/Textarea.js +++ b/packages/form-js-viewer/src/render/components/form-fields/Textarea.js @@ -38,7 +38,7 @@ export default function Textarea(props) { field, value: target.value }); - }, [ props.onChange ]); + }); const onInputBlur = () => { flushOnChange && flushOnChange(); diff --git a/packages/form-js-viewer/src/render/components/form-fields/Textfield.js b/packages/form-js-viewer/src/render/components/form-fields/Textfield.js index 0ad91ed4b..4e1b02185 100644 --- a/packages/form-js-viewer/src/render/components/form-fields/Textfield.js +++ b/packages/form-js-viewer/src/render/components/form-fields/Textfield.js @@ -42,7 +42,7 @@ export default function Textfield(props) { field, value: target.value }); - }, [ props.onChange ]); + }); const onInputBlur = () => { flushOnChange && flushOnChange(); diff --git a/packages/form-js-viewer/src/render/components/form-fields/parts/Datepicker.js b/packages/form-js-viewer/src/render/components/form-fields/parts/Datepicker.js index 7f53c711b..2179b3f35 100644 --- a/packages/form-js-viewer/src/render/components/form-fields/parts/Datepicker.js +++ b/packages/form-js-viewer/src/render/components/form-fields/parts/Datepicker.js @@ -6,6 +6,7 @@ import { useCallback, useEffect, useRef, useState } from 'preact/hooks'; import CalendarIcon from '../icons/Calendar.svg'; import InputAdorner from './InputAdorner'; import Label from '../../Label'; +import { useDeepCompareState } from '../../../hooks'; export default function Datepicker(props) { @@ -18,7 +19,7 @@ export default function Datepicker(props) { required, disabled, disallowPassedDates, - date, + date: dateObject, readonly, setDate } = props; @@ -30,6 +31,9 @@ export default function Datepicker(props) { const [ isInputDirty, setIsInputDirty ] = useState(false); const [ forceFocusCalendar, setForceFocusCalendar ] = useState(false); + // ensures we render based on date value instead of reference + const date = useDeepCompareState(dateObject, null); + // shorts the date value back to the source useEffect(() => { @@ -38,7 +42,7 @@ export default function Datepicker(props) { flatpickrInstance.setDate(date, true); setIsInputDirty(false); - }, [ flatpickrInstance, date.toString() ]); + }, [ flatpickrInstance, date ]); useEffect(() => { diff --git a/packages/form-js-viewer/src/render/components/form-fields/parts/SimpleSelect.js b/packages/form-js-viewer/src/render/components/form-fields/parts/SimpleSelect.js index b2521308b..1956e767e 100644 --- a/packages/form-js-viewer/src/render/components/form-fields/parts/SimpleSelect.js +++ b/packages/form-js-viewer/src/render/components/form-fields/parts/SimpleSelect.js @@ -53,7 +53,7 @@ export default function SimpleSelect(props) { ds.displayCross = ds.componentReady && value !== null && value !== undefined; ds.displayDropdown = !disabled && !readonly && isDropdownExpanded; return ds; - }, [ disabled, isDropdownExpanded, loadState, value ]); + }, [ disabled, isDropdownExpanded, loadState, readonly, value ]); const onMouseDown = useCallback((e) => { const input = inputRef.current; diff --git a/packages/form-js-viewer/src/render/hooks/useCleanupMultiSelectValues.js b/packages/form-js-viewer/src/render/hooks/useCleanupMultiSelectValues.js index 5c8bbde32..e23ea063a 100644 --- a/packages/form-js-viewer/src/render/hooks/useCleanupMultiSelectValues.js +++ b/packages/form-js-viewer/src/render/hooks/useCleanupMultiSelectValues.js @@ -1,6 +1,7 @@ import { useEffect } from 'preact/hooks'; import { LOAD_STATES } from './useOptionsAsync'; import { hasEqualValue } from '../components/util/sanitizerUtil'; +import useDeepCompareState from './useDeepCompareState'; export default function(props) { @@ -12,7 +13,9 @@ export default function(props) { values } = props; - // Ensures that the values are always a subset of the possible options + const memoizedValues = useDeepCompareState(values, []); + + // ensures that the values are always a subset of the possible options useEffect(() => { if (loadState !== LOAD_STATES.LOADED) { @@ -20,15 +23,15 @@ export default function(props) { } const optionValues = options.map(o => o.value); - const hasValuesNotInOptions = values.some(v => !hasEqualValue(v, optionValues)); + const hasValuesNotInOptions = memoizedValues.some(v => !hasEqualValue(v, optionValues)); if (hasValuesNotInOptions) { onChange({ field, - value: values.filter(v => hasEqualValue(v, optionValues)) + value: memoizedValues.filter(v => hasEqualValue(v, optionValues)) }); } - }, [ field, options, onChange, JSON.stringify(values), loadState ]); + }, [ field, options, onChange, memoizedValues, loadState ]); } \ No newline at end of file diff --git a/packages/form-js-viewer/src/render/hooks/useDeepCompareState.js b/packages/form-js-viewer/src/render/hooks/useDeepCompareState.js index b33e3598c..9a60de252 100644 --- a/packages/form-js-viewer/src/render/hooks/useDeepCompareState.js +++ b/packages/form-js-viewer/src/render/hooks/useDeepCompareState.js @@ -4,6 +4,7 @@ import { } from 'preact/hooks'; import usePrevious from './usePrevious'; +import isEqual from 'lodash/isEqual'; /** * A custom hook to manage state changes with deep comparison. @@ -16,9 +17,9 @@ export default function useDeepCompareState(value, defaultValue) { const [ state, setState ] = useState(defaultValue); - const previous = usePrevious(value, defaultValue, [ value ]); + const previous = usePrevious(value, defaultValue); - const changed = !compare(previous, value); + const changed = !isEqual(previous, value); useEffect(() => { if (changed) { @@ -27,11 +28,4 @@ export default function useDeepCompareState(value, defaultValue) { }, [ changed, value ]); return state; - -} - -// helpers ////////////////////////// - -function compare(a, b) { - return JSON.stringify(a) === JSON.stringify(b); -} +} \ No newline at end of file diff --git a/packages/form-js-viewer/src/render/hooks/useFlushDebounce.js b/packages/form-js-viewer/src/render/hooks/useFlushDebounce.js index cccaafafa..f2e7c7dc1 100644 --- a/packages/form-js-viewer/src/render/hooks/useFlushDebounce.js +++ b/packages/form-js-viewer/src/render/hooks/useFlushDebounce.js @@ -1,7 +1,7 @@ import { useCallback, useRef } from 'preact/hooks'; import useService from './useService'; -function useFlushDebounce(func, additionalDeps = []) { +function useFlushDebounce(func) { const timeoutRef = useRef(null); const lastArgsRef = useRef(null); @@ -29,7 +29,7 @@ function useFlushDebounce(func, additionalDeps = []) { lastArgsRef.current = null; }, delay); - }, [ func, delay, shouldDebounce, ...additionalDeps ]); + }, [ func, delay, shouldDebounce ]); const flushFunc = useCallback(() => { @@ -42,7 +42,7 @@ function useFlushDebounce(func, additionalDeps = []) { timeoutRef.current = null; } - }, [ func, ...additionalDeps ]); + }, [ func ]); return [ debounceFunc, flushFunc ]; } diff --git a/packages/form-js-viewer/src/render/hooks/usePrevious.js b/packages/form-js-viewer/src/render/hooks/usePrevious.js index f5c6d0766..21a3bfbf1 100644 --- a/packages/form-js-viewer/src/render/hooks/usePrevious.js +++ b/packages/form-js-viewer/src/render/hooks/usePrevious.js @@ -4,10 +4,10 @@ import { } from 'preact/hooks'; -export default function usePrevious(value, defaultValue, dependencies) { +export default function usePrevious(value, defaultValue = null) { const ref = useRef(defaultValue); - useEffect(() => ref.current = value, dependencies); + useEffect(() => ref.current = value, [ value ]); return ref.current; } \ No newline at end of file diff --git a/packages/form-js-viewer/test/helper/preactDebuggers.js b/packages/form-js-viewer/test/helper/preactDebuggers.js index 821261e74..cd81b95de 100644 --- a/packages/form-js-viewer/test/helper/preactDebuggers.js +++ b/packages/form-js-viewer/test/helper/preactDebuggers.js @@ -4,11 +4,11 @@ const usePrevious = (value, initialValue) => { const ref = useRef(initialValue); useEffect(() => { ref.current = value; - }); + }, [ value ]); return ref.current; }; -export function useEffectDebugger(effectHook, dependencies, dependencyNames = [], effectName = 'noname') { +export function useEffectDebugger(effect, dependencies, dependencyNames = [], effectName = 'noname') { const previousDeps = usePrevious(dependencies, []); const changedDeps = dependencies.reduce((accum, dependency, index) => { @@ -30,7 +30,7 @@ export function useEffectDebugger(effectHook, dependencies, dependencyNames = [] console.log('[use-effect-debugger] (' + effectName + ') ', changedDeps); } - useEffect(effectHook, dependencies); + useEffect(effect, [ effect, ...dependencies ]); } export function useCallbackDebugger(callback, dependencies, dependencyNames = [], callbackName = 'noname') { @@ -55,5 +55,5 @@ export function useCallbackDebugger(callback, dependencies, dependencyNames = [] console.log('[use-callback-debugger] (' + callbackName + ') ', changedDeps); } - return useCallback(callback, dependencies); + return useCallback(callback, [ callback, ...dependencies ]); } \ No newline at end of file