From 1622816035c98e434373c64268c8e21b2c4cfa02 Mon Sep 17 00:00:00 2001 From: Eric NGUYEN Date: Tue, 16 Aug 2022 16:34:32 +0200 Subject: [PATCH 1/4] Final fix for XPositionReference : We are now lying to the user in the form. The transformation is applied to the value that is shown but the transformation is restored for the computing afterward --- src/Components/Editor/ContainerOperations.ts | 11 ++---- src/Components/Editor/PropertiesOperations.ts | 36 +++++++++++++++---- src/Components/Properties/DynamicForm.tsx | 13 +++---- src/Components/Properties/StaticForm.tsx | 3 +- src/Components/SVG/Elements/Container.tsx | 30 +++++++++------- src/Components/SVG/Elements/Selector.tsx | 11 ++---- 6 files changed, 59 insertions(+), 45 deletions(-) diff --git a/src/Components/Editor/ContainerOperations.ts b/src/Components/Editor/ContainerOperations.ts index 95a0889..28af9e7 100644 --- a/src/Components/Editor/ContainerOperations.ts +++ b/src/Components/Editor/ContainerOperations.ts @@ -7,7 +7,7 @@ import { getCurrentHistory } from './Editor'; import IProperties from '../../Interfaces/IProperties'; import { AddMethod } from '../../Enums/AddMethod'; import { IAvailableContainer } from '../../Interfaces/IAvailableContainer'; -import { transformPosition } from '../SVG/Elements/Container'; +import { transformX } from '../SVG/Elements/Container'; import { XPositionReference } from '../../Enums/XPositionReference'; /** @@ -267,14 +267,7 @@ function ApplyAddMethod(index: number, containerConfig: IAvailableContainer, par const lastChild: IContainerModel | undefined = parent.children.at(index - 1); if (lastChild !== undefined) { - const [transformedX] = transformPosition( - lastChild.properties.x, - lastChild.properties.y, - lastChild.properties.width, - lastChild.properties.XPositionReference - ); - - x += transformedX + lastChild.properties.width; + x += (lastChild.properties.x + lastChild.properties.width); } } return x; diff --git a/src/Components/Editor/PropertiesOperations.ts b/src/Components/Editor/PropertiesOperations.ts index 9f35e63..59098a6 100644 --- a/src/Components/Editor/PropertiesOperations.ts +++ b/src/Components/Editor/PropertiesOperations.ts @@ -6,6 +6,7 @@ import { getCurrentHistory } from './Editor'; import { RecalculatePhysics } from './Behaviors/RigidBodyBehaviors'; import { INPUT_TYPES } from '../Properties/PropertiesInputTypes'; import { ImposePosition } from './Behaviors/AnchorBehaviors'; +import { restoreX } from '../SVG/Elements/Container'; /** * Handled the property change event in the properties form @@ -40,11 +41,7 @@ export function OnPropertyChange( if (isStyle) { (container.properties.style as any)[key] = value; } else { - if (INPUT_TYPES[key] === 'number') { - (container.properties as any)[key] = Number(value); - } else { - (container.properties as any)[key] = value; - } + (container.properties as any)[key] = value; } if (container.properties.isAnchor) { @@ -104,10 +101,33 @@ export function OnPropertiesSubmit( } if (input instanceof HTMLInputElement) { - (container.properties as any)[property] = input.value; if (INPUT_TYPES[property] === 'number') { + if (property === 'x') { + // Hardcoded fix for XPositionReference + + const inputWidth: HTMLInputElement | null = (event.target as HTMLFormElement).querySelector('#width'); + const inputRadio: HTMLDivElement | null = (event.target as HTMLFormElement).querySelector('#XPositionReference'); + + if (inputWidth === null || inputRadio === null) { + throw new Error('[OnPropertiesSubmit] Missing inputs for width or XPositionReference'); + } + + const radiobutton: HTMLInputElement | null = inputRadio.querySelector('input[name="XPositionReference"]:checked'); + + if (radiobutton === null) { + throw new Error('[OnPropertiesSubmit] Missing inputs for XPositionReference'); + } + + const x = restoreX(Number(input.value), Number(inputWidth.value), Number(radiobutton.value)); + (container.properties as any)[property] = x; + continue; + } + (container.properties as any)[property] = Number(input.value); + continue; } + + (container.properties as any)[property] = input.value; } else if (input instanceof HTMLDivElement) { const radiobutton: HTMLInputElement | null = input.querySelector(`input[name="${property}"]:checked`); @@ -115,10 +135,12 @@ export function OnPropertiesSubmit( continue; } - (container.properties as any)[property] = radiobutton.value; if (INPUT_TYPES[property] === 'number') { (container.properties as any)[property] = Number(radiobutton.value); + continue; } + + (container.properties as any)[property] = radiobutton.value; } } diff --git a/src/Components/Properties/DynamicForm.tsx b/src/Components/Properties/DynamicForm.tsx index 0be155d..b5665b6 100644 --- a/src/Components/Properties/DynamicForm.tsx +++ b/src/Components/Properties/DynamicForm.tsx @@ -4,6 +4,7 @@ import { XPositionReference } from '../../Enums/XPositionReference'; import IProperties from '../../Interfaces/IProperties'; import { InputGroup } from '../InputGroup/InputGroup'; import { RadioGroupButtons } from '../RadioGroupButtons/RadioGroupButtons'; +import { restoreX, transformX } from '../SVG/Elements/Container'; interface IDynamicFormProps { properties: IProperties @@ -57,8 +58,8 @@ const DynamicForm: React.FunctionComponent = (props) => { labelClassName='' inputClassName='' type='number' - value={props.properties.x.toString()} - onChange={(event) => props.onChange('x', event.target.value)} + value={transformX(props.properties.x, props.properties.width, props.properties.XPositionReference).toString()} + onChange={(event) => props.onChange('x', restoreX(Number(event.target.value), props.properties.width, props.properties.XPositionReference))} /> = (props) => { inputClassName='' type='number' value={props.properties.y.toString()} - onChange={(event) => props.onChange('y', event.target.value)} + onChange={(event) => props.onChange('y', Number(event.target.value))} /> = (props) => { inputClassName='' type='number' value={props.properties.width.toString()} - onChange={(event) => props.onChange('width', event.target.value)} + onChange={(event) => props.onChange('width', Number(event.target.value))} /> = (props) => { inputClassName='' type='number' value={props.properties.height.toString()} - onChange={(event) => props.onChange('height', event.target.value)} + onChange={(event) => props.onChange('height', Number(event.target.value))} /> = (props) => { value: XPositionReference.Right.toString() } ]} - onChange={(event) => props.onChange('XPositionReference', event.target.value)} + onChange={(event) => props.onChange('XPositionReference', Number(event.target.value))} /> { getCSSInputs(props.properties, props.onChange) } diff --git a/src/Components/Properties/StaticForm.tsx b/src/Components/Properties/StaticForm.tsx index f16d196..9f6055b 100644 --- a/src/Components/Properties/StaticForm.tsx +++ b/src/Components/Properties/StaticForm.tsx @@ -4,6 +4,7 @@ import { XPositionReference } from '../../Enums/XPositionReference'; import IProperties from '../../Interfaces/IProperties'; import { InputGroup } from '../InputGroup/InputGroup'; import { RadioGroupButtons } from '../RadioGroupButtons/RadioGroupButtons'; +import { transformX } from '../SVG/Elements/Container'; interface IStaticFormProps { properties: IProperties @@ -57,7 +58,7 @@ const StaticForm: React.FunctionComponent = (props) => { labelClassName='' inputClassName='' type='number' - defaultValue={props.properties.x.toString()} + defaultValue={transformX(props.properties.x, props.properties.width, props.properties.XPositionReference).toString()} /> = (props: IContainerProps) => const xText = props.model.properties.width / 2; const yText = props.model.properties.height / 2; - const [transformedX, transformedY] = transformPosition( - props.model.properties.x, - props.model.properties.y, - props.model.properties.width, - props.model.properties.XPositionReference - ); - const transform = `translate(${transformedX}, ${transformedY})`; + const transform = `translate(${props.model.properties.x}, ${props.model.properties.y})`; // g style const defaultStyle: React.CSSProperties = { @@ -107,17 +101,17 @@ function GetChildrenDimensionProps(props: IContainerProps, dimensionMargin: numb const childrenId = `dim-children-${props.model.properties.id}`; const lastChild = props.model.children[props.model.children.length - 1]; - let xChildrenStart = lastChild.properties.x; - let xChildrenEnd = lastChild.properties.x; + let xChildrenStart = transformX(lastChild.properties.x, lastChild.properties.width, lastChild.properties.XPositionReference); + let xChildrenEnd = transformX(lastChild.properties.x, lastChild.properties.width, lastChild.properties.XPositionReference); // Find the min and max for (let i = props.model.children.length - 2; i >= 0; i--) { const child = props.model.children[i]; - const left = child.properties.x; + const left = transformX(child.properties.x, child.properties.width, child.properties.XPositionReference); if (left < xChildrenStart) { xChildrenStart = left; } - const right = child.properties.x; + const right = transformX(child.properties.x, child.properties.width, child.properties.XPositionReference); if (right > xChildrenEnd) { xChildrenEnd = right; } @@ -128,12 +122,22 @@ function GetChildrenDimensionProps(props: IContainerProps, dimensionMargin: numb return { childrenId, xChildrenStart, xChildrenEnd, yChildren, textChildren }; } -export function transformPosition(x: number, y: number, width: number, xPositionReference = XPositionReference.Left): [number, number] { +export function transformX(x: number, width: number, xPositionReference = XPositionReference.Left): number { + let transformedX = x; + if (xPositionReference === XPositionReference.Center) { + transformedX += width / 2; + } else if (xPositionReference === XPositionReference.Right) { + transformedX += width; + } + return transformedX; +} + +export function restoreX(x: number, width: number, xPositionReference = XPositionReference.Left): number { let transformedX = x; if (xPositionReference === XPositionReference.Center) { transformedX -= width / 2; } else if (xPositionReference === XPositionReference.Right) { transformedX -= width; } - return [transformedX, y]; + return transformedX; } diff --git a/src/Components/SVG/Elements/Selector.tsx b/src/Components/SVG/Elements/Selector.tsx index 101f19a..e70ca79 100644 --- a/src/Components/SVG/Elements/Selector.tsx +++ b/src/Components/SVG/Elements/Selector.tsx @@ -1,7 +1,6 @@ import * as React from 'react'; import { IContainerModel } from '../../../Interfaces/IContainerModel'; import { getAbsolutePosition } from '../../../utils/itertools'; -import { transformPosition } from './Container'; interface ISelectorProps { selected: IContainerModel | null @@ -16,12 +15,6 @@ export const Selector: React.FC = (props) => { } const [x, y] = getAbsolutePosition(props.selected); - const [transformedX, transformedY] = transformPosition( - x, - y, - props.selected.properties.width, - props.selected.properties.XPositionReference - ); const [width, height] = [ props.selected.properties.width, props.selected.properties.height @@ -38,8 +31,8 @@ export const Selector: React.FC = (props) => { return ( Date: Tue, 16 Aug 2022 16:34:44 +0200 Subject: [PATCH 2/4] History: Fix regression with delete --- src/Components/Editor/ContainerOperations.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/Components/Editor/ContainerOperations.ts b/src/Components/Editor/ContainerOperations.ts index 28af9e7..3d9e91e 100644 --- a/src/Components/Editor/ContainerOperations.ts +++ b/src/Components/Editor/ContainerOperations.ts @@ -7,7 +7,6 @@ import { getCurrentHistory } from './Editor'; import IProperties from '../../Interfaces/IProperties'; import { AddMethod } from '../../Enums/AddMethod'; import { IAvailableContainer } from '../../Interfaces/IAvailableContainer'; -import { transformX } from '../SVG/Elements/Container'; import { XPositionReference } from '../../Enums/XPositionReference'; /** @@ -58,7 +57,7 @@ export function DeleteContainer( setHistoryCurrentStep: Dispatch> ): void { const history = getCurrentHistory(fullHistory, historyCurrentStep); - const current = history[historyCurrentStep]; + const current = history[history.length - 1]; const mainContainerClone: IContainerModel = structuredClone(current.MainContainer); const container = findContainerById(mainContainerClone, containerId); -- 2.47.2 From e3190dfe0a129a21b3757ba8edc4e5fc93581ea7 Mon Sep 17 00:00:00 2001 From: Eric NGUYEN Date: Tue, 16 Aug 2022 16:48:43 +0200 Subject: [PATCH 3/4] Refactor PropertiesOperations for more readability --- src/Components/Editor/PropertiesOperations.ts | 120 +++++++++++------- 1 file changed, 74 insertions(+), 46 deletions(-) diff --git a/src/Components/Editor/PropertiesOperations.ts b/src/Components/Editor/PropertiesOperations.ts index 59098a6..0e3802c 100644 --- a/src/Components/Editor/PropertiesOperations.ts +++ b/src/Components/Editor/PropertiesOperations.ts @@ -3,7 +3,7 @@ import { IContainerModel, ContainerModel } from '../../Interfaces/IContainerMode import { IHistoryState } from '../../Interfaces/IHistoryState'; import { findContainerById } from '../../utils/itertools'; import { getCurrentHistory } from './Editor'; -import { RecalculatePhysics } from './Behaviors/RigidBodyBehaviors'; +import { constraintBodyInsideUnallocatedWidth, RecalculatePhysics } from './Behaviors/RigidBodyBehaviors'; import { INPUT_TYPES } from '../Properties/PropertiesInputTypes'; import { ImposePosition } from './Behaviors/AnchorBehaviors'; import { restoreX } from '../SVG/Elements/Container'; @@ -93,64 +93,28 @@ export function OnPropertiesSubmit( } // Assign container properties + const form: HTMLFormElement = event.target as HTMLFormElement; for (const property in container.properties) { - const input: HTMLInputElement | HTMLDivElement | null = (event.target as HTMLFormElement).querySelector(`#${property}`); + const input: HTMLInputElement | HTMLDivElement | null = form.querySelector(`#${property}`); if (input === null) { continue; } if (input instanceof HTMLInputElement) { - if (INPUT_TYPES[property] === 'number') { - if (property === 'x') { - // Hardcoded fix for XPositionReference + submitHTMLInput(input, container, property, form); + continue; + } - const inputWidth: HTMLInputElement | null = (event.target as HTMLFormElement).querySelector('#width'); - const inputRadio: HTMLDivElement | null = (event.target as HTMLFormElement).querySelector('#XPositionReference'); - - if (inputWidth === null || inputRadio === null) { - throw new Error('[OnPropertiesSubmit] Missing inputs for width or XPositionReference'); - } - - const radiobutton: HTMLInputElement | null = inputRadio.querySelector('input[name="XPositionReference"]:checked'); - - if (radiobutton === null) { - throw new Error('[OnPropertiesSubmit] Missing inputs for XPositionReference'); - } - - const x = restoreX(Number(input.value), Number(inputWidth.value), Number(radiobutton.value)); - (container.properties as any)[property] = x; - continue; - } - - (container.properties as any)[property] = Number(input.value); - continue; - } - - (container.properties as any)[property] = input.value; - } else if (input instanceof HTMLDivElement) { - const radiobutton: HTMLInputElement | null = input.querySelector(`input[name="${property}"]:checked`); - - if (radiobutton === null) { - continue; - } - - if (INPUT_TYPES[property] === 'number') { - (container.properties as any)[property] = Number(radiobutton.value); - continue; - } - - (container.properties as any)[property] = radiobutton.value; + if (input instanceof HTMLDivElement) { + submitRadioButtons(input, container, property); + continue; } } // Assign cssproperties for (const styleProperty in container.properties.style) { - const input: HTMLInputElement | null = (event.target as HTMLFormElement).querySelector(`#${styleProperty}`); - if (input === null) { - continue; - } - (container.properties.style as any)[styleProperty] = input.value; + submitCSSForm(form, styleProperty, container); } if (container.properties.isRigidBody) { @@ -167,3 +131,67 @@ export function OnPropertiesSubmit( setHistory(history); setHistoryCurrentStep(history.length - 1); } + +const submitHTMLInput = ( + input: HTMLInputElement, + container: IContainerModel, + property: string, + form: HTMLFormElement +): void => { + if (INPUT_TYPES[property] !== 'number') { + (container.properties as any)[property] = input.value; + } + + if (property === 'x') { + // Hardcoded fix for XPositionReference + const x = RestoreX(form, input); + (container.properties as any)[property] = x; + return; + } + + (container.properties as any)[property] = Number(input.value); +}; + +const submitCSSForm = (form: HTMLFormElement, styleProperty: string, container: ContainerModel): void => { + const input: HTMLInputElement | null = form.querySelector(`#${styleProperty}`); + if (input === null) { + return; + } + (container.properties.style as any)[styleProperty] = input.value; +}; + +const RestoreX = ( + form: HTMLFormElement, + input: HTMLInputElement +): number => { + const inputWidth: HTMLInputElement | null = form.querySelector('#width'); + const inputRadio: HTMLDivElement | null = form.querySelector('#XPositionReference'); + if (inputWidth === null || inputRadio === null) { + throw new Error('[OnPropertiesSubmit] Missing inputs for width or XPositionReference'); + } + + const radiobutton: HTMLInputElement | null = inputRadio.querySelector('input[name="XPositionReference"]:checked'); + if (radiobutton === null) { + throw new Error('[OnPropertiesSubmit] Missing inputs for XPositionReference'); + } + + return restoreX(Number(input.value), Number(inputWidth.value), Number(radiobutton.value)); +}; + +const submitRadioButtons = ( + div: HTMLDivElement, + container: IContainerModel, + property: string +): void => { + const radiobutton: HTMLInputElement | null = div.querySelector(`input[name="${property}"]:checked`); + if (radiobutton === null) { + return; + } + + if (INPUT_TYPES[property] === 'number') { + (container.properties as any)[property] = Number(radiobutton.value); + return; + } + + (container.properties as any)[property] = radiobutton.value; +}; -- 2.47.2 From a36d5d79d498b4d70d77245c5b3f94305b44e319 Mon Sep 17 00:00:00 2001 From: Eric NGUYEN Date: Tue, 16 Aug 2022 16:54:12 +0200 Subject: [PATCH 4/4] Fix test: when using dynamic input, the value is converted correctly as number before being send to the change event --- src/Components/Properties/Properties.test.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Components/Properties/Properties.test.tsx b/src/Components/Properties/Properties.test.tsx index 61441f8..dc9473e 100644 --- a/src/Components/Properties/Properties.test.tsx +++ b/src/Components/Properties/Properties.test.tsx @@ -68,8 +68,8 @@ describe.concurrent('Properties', () => { expect(prop.id).toBe('stuff'); expect(prop.parentId).toBe('parentId'); - expect(prop.x).toBe('2'); - expect(prop.y).toBe('2'); + expect(prop.x).toBe(2); + expect(prop.y).toBe(2); rerender(