Fix: Enhance position values validation [APP-1009]

elementor · Jan 22, 2025 · d42bce5 · d42bce5
1 parent 034d684
commit d42bce5
Show file tree

Hide file tree

Showing 4 changed files with 85 additions and 52 deletions.
diff --git a/modules/settings/assets/js/components/bottom-bar/index.js b/modules/settings/assets/js/components/bottom-bar/index.js
@@ -17,47 +17,39 @@ const BottomBar = () => {
 	const { success, error } = useToastNotification();
 
 	const saveSettings = async () => {
+		let savedData = {};
+
 		if (selectedMenu.parent === 'widget' && selectedMenu.child === 'menu') {
-			try {
-				await save({
-					ea11y_widget_menu_settings: widgetMenuSettings,
-				});
-				success('Settings saved!');
-				setHasChanges(false);
-				mixpanelService.sendEvent('save_button_clicked', {
-					savedData: {
-						ea11y_widget_menu_settings: widgetMenuSettings,
-					},
-				});
-			} catch (e) {
-				error('Failed to save settings!');
-			}
+			savedData = {
+				ea11y_widget_menu_settings: widgetMenuSettings,
+			};
 		} else if (
 			selectedMenu.parent === 'widget' &&
 			selectedMenu.child === 'iconSettings'
 		) {
-			try {
-				await save({
-					ea11y_widget_icon_settings: {
-						style: iconDesign,
-						position: iconPosition,
-					},
-				});
+			savedData = {
+				ea11y_widget_icon_settings: {
+					style: iconDesign,
+					position: iconPosition,
+				},
+			};
+		}
+
+		try {
+			await save(savedData);
 
-				success('Settings saved!');
-				setHasChanges(false);
+			success(__('Settings saved!', 'pojo-accessibility'));
 
-				mixpanelService.sendEvent('save_button_clicked', {
-					savedData: {
-						style: iconDesign,
-						position: iconPosition,
-					},
-				});
-			} catch (e) {
-				error('Failed to save settings!');
-			}
+			setHasChanges(false);
+
+			mixpanelService.sendEvent('save_button_clicked', {
+				savedData,
+			});
+		} catch (e) {
+			error(__('Failed to save settings!', 'pojo-accessibility'));
 		}
 	};
+
 	return (
 		<Box
 			display="flex"
@@ -72,7 +64,7 @@ const BottomBar = () => {
 				onClick={saveSettings}
 				disabled={!hasChanges}
 			>
-				{__('Save Changes', 'pojo-accessibility')}
+				{__('Save changes', 'pojo-accessibility')}
 			</Button>
 		</Box>
 	);

diff --git a/modules/settings/assets/js/components/position-control/index.js b/modules/settings/assets/js/components/position-control/index.js
@@ -28,13 +28,23 @@ const verticalOptions = [
 	{ value: 'bottom', label: __('Lower', 'pojo-accessibility') },
 ];
 
-// Customization for the WP admin global CSS.
+const StyledContainer = styled(Box)`
+	display: flex;
+	gap: ${({ theme }) => theme.spacing(1)};
+	margin-top: ${({ theme }) => theme.spacing(2)};
+	margin-bottom: ${({ theme, isError }) =>
+		isError ? theme.spacing(4) : 'initial'};
+
+	transition: all 100ms ease-in-out;
+`;
+
 const StyledTextField = styled(TextField)`
 	width: 200px;
 	height: 56px;
 
 	.wp-admin & .MuiInputBase-input,
 	& .MuiInputBase-input:focus {
+		height: 56px;
 		background-color: initial;
 		box-shadow: none;
 		border: 0;
@@ -45,28 +55,34 @@ const StyledTextField = styled(TextField)`
 		&.MuiInputBase-inputSizeSmall {
 			padding: 8.5px 14px 8.5px 14px;
 		}
-		height: 56px;
 	}
 `;
 
 const PositionControl = ({ type, disabled, mode }) => {
 	const { iconPosition, updateExactPosition } = useIconPosition();
 	const [unitsIndex, setUnitsIndex] = useState(0);
+	const [inputValue, setInputValue] = useState(
+		iconPosition[mode]?.exactPosition[type]?.value,
+	);
+	const [isValid, setIsValid] = useState(inputValue >= 5 && inputValue <= 550);
 	const popupState = usePopupState({
 		variant: 'popover',
 		popupId: 'position-settings',
 	});
 
 	const handleMenuItemClick = (index) => {
 		setUnitsIndex(index);
+
 		updateExactPosition(
 			mode,
 			type,
 			iconPosition[mode]?.exactPosition[type]?.direction,
 			iconPosition[mode]?.exactPosition[type]?.value,
 			units[index],
 		);
+
 		popupState.close();
+
 		mixpanelService.sendEvent('handle_unit_changed', {
 			positionData: {
 				mode,
@@ -79,22 +95,31 @@ const PositionControl = ({ type, disabled, mode }) => {
 	};
 
 	const handlePositionChange = (event) => {
-		updateExactPosition(
-			mode,
-			type,
-			iconPosition[mode]?.exactPosition[type]?.direction,
-			event.target.value,
-			iconPosition[mode]?.exactPosition[type]?.unit,
-		);
-		mixpanelService.sendEvent('handle_value_changed', {
-			positionData: {
+		const value = parseInt(event.target.value, 10) || 0;
+		const valueIsValid = value >= 5 && value <= 550;
+
+		setInputValue(event.target.value);
+		setIsValid(valueIsValid);
+
+		if (valueIsValid) {
+			updateExactPosition(
 				mode,
 				type,
-				value: event.target.value,
-				unit: iconPosition[mode]?.exactPosition[type]?.unit,
-				direction: iconPosition[mode]?.exactPosition[type]?.value,
-			},
-		});
+				iconPosition[mode]?.exactPosition[type]?.direction,
+				value,
+				iconPosition[mode]?.exactPosition[type]?.unit,
+			);
+
+			mixpanelService.sendEvent('handle_value_changed', {
+				positionData: {
+					mode,
+					type,
+					value,
+					unit: iconPosition[mode]?.exactPosition[type]?.unit,
+					direction: iconPosition[mode]?.exactPosition[type]?.value,
+				},
+			});
+		}
 	};
 
 	const handlePositionDirection = (event) => {
@@ -105,6 +130,7 @@ const PositionControl = ({ type, disabled, mode }) => {
 			iconPosition[mode]?.exactPosition[type]?.value,
 			iconPosition[mode]?.exactPosition[type]?.unit,
 		);
+
 		mixpanelService.sendEvent('handle_direction_changed', {
 			positionData: {
 				mode,
@@ -117,11 +143,13 @@ const PositionControl = ({ type, disabled, mode }) => {
 	};
 
 	return (
-		<Box display="flex" gap={1} marginTop={2}>
+		<StyledContainer isError={!isValid}>
 			<StyledTextField
 				size="medium"
+				error={!isValid}
+				helperText={!isValid ? 'Invalid value' : ''}
 				disabled={disabled}
-				value={iconPosition[mode]?.exactPosition?.[type].value}
+				value={inputValue}
 				onChange={handlePositionChange}
 				InputProps={{
 					endAdornment: (
@@ -153,6 +181,7 @@ const PositionControl = ({ type, disabled, mode }) => {
 
 			<Select
 				fullWidth
+				variant="outlined"
 				onChange={handlePositionDirection}
 				disabled={disabled}
 				value={iconPosition[mode]?.exactPosition?.[type].direction}
@@ -184,7 +213,7 @@ const PositionControl = ({ type, disabled, mode }) => {
 							</MenuItem>
 						))}
 			</Select>
-		</Box>
+		</StyledContainer>
 	);
 };
 

diff --git a/modules/settings/assets/js/contexts/plugin-settings-context.js b/modules/settings/assets/js/contexts/plugin-settings-context.js
diff --git a/modules/settings/assets/js/layouts/position-settings-desktop.js b/modules/settings/assets/js/layouts/position-settings-desktop.js
@@ -12,6 +12,7 @@ const PositionSettingsDesktop = () => {
 
 	const toggleVisibility = (device) => {
 		updateIconPosition(device, 'hidden', !iconPosition[device].hidden);
+
 		mixpanelService.sendEvent('toggle_clicked', {
 			toggleData: {
 				state: !iconPosition[device].hidden,
@@ -27,6 +28,7 @@ const PositionSettingsDesktop = () => {
 			'enableExactPosition',
 			!iconPosition[device].enableExactPosition,
 		);
+
 		mixpanelService.sendEvent('toggle_clicked', {
 			toggleData: {
 				state: !iconPosition[device].enableExactPosition,
@@ -67,6 +69,7 @@ const PositionSettingsDesktop = () => {
 					gap={5}
 				>
 					<AlignmentMatrixControl mode="desktop" />
+
 					<Box>
 						<FormControlLabel
 							label={exactPositionLabel}
@@ -76,11 +79,20 @@ const PositionSettingsDesktop = () => {
 							onChange={() => toggleExactPosition('desktop')}
 							checked={iconPosition.desktop?.enableExactPosition}
 						/>
+
+						<Typography variant="body2" sx={{ marginTop: 2, marginBottom: 1 }}>
+							{__(
+								'Exact positioning, 5 – 500 px are permitted values:',
+								'pojo-accessibility',
+							)}
+						</Typography>
+
 						<PositionControl
 							type="horizontal"
 							mode="desktop"
 							disabled={!iconPosition.desktop?.enableExactPosition}
 						/>
+
 						<PositionControl
 							type="vertical"
 							mode="desktop"