1
0
mirror of https://github.com/laurent22/joplin.git synced 2025-01-11 18:24:43 +02:00

Remove slider component mode and replace integer settings with validated text inputs and ensure all validation error flows work correctly

This commit is contained in:
Josh C 2024-11-12 13:17:05 +00:00
parent e16f452bdf
commit dc84c0c546
12 changed files with 1990 additions and 2067 deletions

View File

@ -1,62 +0,0 @@
diff --git a/android/src/newarch/java/com/reactnativecommunity/slider/ReactSliderManager.java b/android/src/newarch/java/com/reactnativecommunity/slider/ReactSliderManager.java
index a5bb95eec3337b93a2338a2869a2bda176c91cae..87817688eb280c2f702c26dc35558c6a0a4db1ea 100644
--- a/android/src/newarch/java/com/reactnativecommunity/slider/ReactSliderManager.java
+++ b/android/src/newarch/java/com/reactnativecommunity/slider/ReactSliderManager.java
@@ -42,12 +42,20 @@ public class ReactSliderManager extends SimpleViewManager<ReactSlider> implement
public void onProgressChanged(SeekBar seekbar, int progress, boolean fromUser) {
ReactSlider slider = (ReactSlider)seekbar;
- if(progress < slider.getLowerLimit()) {
- progress = slider.getLowerLimit();
- seekbar.setProgress(progress);
- } else if (progress > slider.getUpperLimit()) {
- progress = slider.getUpperLimit();
- seekbar.setProgress(progress);
+ // During initialization, lowerLimit can be greater than upperLimit.
+ //
+ // If a change event is received during this, we need a check to prevent
+ // infinite recursion.
+ //
+ // Issue: https://github.com/callstack/react-native-slider/issues/571
+ if (slider.getLowerLimit() <= slider.getUpperLimit()) {
+ if(progress < slider.getLowerLimit()) {
+ progress = slider.getLowerLimit();
+ seekbar.setProgress(progress);
+ } else if (progress > slider.getUpperLimit()) {
+ progress = slider.getUpperLimit();
+ seekbar.setProgress(progress);
+ }
}
ReactContext reactContext = (ReactContext) seekbar.getContext();
diff --git a/android/src/oldarch/java/com/reactnativecommunity/slider/ReactSliderManager.java b/android/src/oldarch/java/com/reactnativecommunity/slider/ReactSliderManager.java
index 3ff5930f85a3cd92c2549925f41058abb188a57e..ab3681fdfe0b736c97020e1434e450c8183e6f18 100644
--- a/android/src/oldarch/java/com/reactnativecommunity/slider/ReactSliderManager.java
+++ b/android/src/oldarch/java/com/reactnativecommunity/slider/ReactSliderManager.java
@@ -30,12 +30,20 @@ public class ReactSliderManager extends SimpleViewManager<ReactSlider> {
public void onProgressChanged(SeekBar seekbar, int progress, boolean fromUser) {
ReactSlider slider = (ReactSlider)seekbar;
- if(progress < slider.getLowerLimit()) {
- progress = slider.getLowerLimit();
- seekbar.setProgress(progress);
- } else if(progress > slider.getUpperLimit()) {
- progress = slider.getUpperLimit();
- seekbar.setProgress(progress);
+ // During initialization, lowerLimit can be greater than upperLimit.
+ //
+ // If a change event is received during this, we need a check to prevent
+ // infinite recursion.
+ //
+ // Issue: https://github.com/callstack/react-native-slider/issues/571
+ if (slider.getLowerLimit() <= slider.getUpperLimit()) {
+ if(progress < slider.getLowerLimit()) {
+ progress = slider.getLowerLimit();
+ seekbar.setProgress(progress);
+ } else if (progress > slider.getUpperLimit()) {
+ progress = slider.getUpperLimit();
+ seekbar.setProgress(progress);
+ }
}
ReactContext reactContext = (ReactContext) seekbar.getContext();

View File

@ -108,7 +108,6 @@
"app-builder-lib@24.4.0": "patch:app-builder-lib@npm%3A24.4.0#./.yarn/patches/app-builder-lib-npm-24.4.0-05322ff057.patch",
"nanoid": "patch:nanoid@npm%3A3.3.7#./.yarn/patches/nanoid-npm-3.3.7-98824ba130.patch",
"pdfjs-dist": "patch:pdfjs-dist@npm%3A3.11.174#./.yarn/patches/pdfjs-dist-npm-3.11.174-67f2fee6d6.patch",
"@react-native-community/slider": "patch:@react-native-community/slider@npm%3A4.4.4#./.yarn/patches/@react-native-community-slider-npm-4.4.4-d78e472f48.patch",
"husky": "patch:husky@npm%3A3.1.0#./.yarn/patches/husky-npm-3.1.0-5cc13e4e34.patch",
"chokidar@^2.0.0": "3.5.3",
"react-native@0.74.1": "patch:react-native@npm%3A0.74.1#./.yarn/patches/react-native-npm-0.74.1-754c02ae9e.patch",

View File

@ -144,14 +144,18 @@ class ConfigScreenComponent extends React.Component<any, any> {
screenName = section.name;
if (this.hasChanges()) {
const ok = confirm(_('This will open a new screen. Save your current changes?'));
const ok = bridge().showConfirmMessageBox(_('This will open a new screen. Save your current changes?'));
if (ok) {
await shared.saveSettings(this);
const result = await shared.saveSettings(this);
if (result === false) return false;
} else {
return false;
}
}
}
this.setState({ selectedSectionName: section.name, screenName: screenName });
return true;
}
// eslint-disable-next-line @typescript-eslint/no-explicit-any -- Old code before rule was applied

View File

@ -130,11 +130,13 @@ class ConfigScreenComponent extends BaseScreenComponent<ConfigScreenProps, Confi
const shouldSetIgnoreTlsErrors = this.state.changedSettingKeys.includes('net.ignoreTlsErrors');
const done = await shared.saveSettings(this);
if (!done) return;
if (!done) return false;
if (shouldSetIgnoreTlsErrors) {
await setIgnoreTlsErrors(Setting.value('net.ignoreTlsErrors'));
}
return true;
};
private syncStatusButtonPress_ = () => {
@ -262,22 +264,21 @@ class ConfigScreenComponent extends BaseScreenComponent<ConfigScreenProps, Confi
return this.state.changedSettingKeys.length > 0;
}
private async promptSaveChanges(): Promise<void> {
private async promptSaveChanges(): Promise<boolean> {
if (this.hasUnsavedChanges()) {
const response = await shim.showMessageBox(_('There are unsaved changes.'), {
buttons: [_('Save changes'), _('Discard changes')],
});
if (response === 0) {
await this.saveButton_press();
return await this.saveButton_press();
}
}
return true;
}
private handleNavigateToNewScreen = async (): Promise<boolean> => {
await this.promptSaveChanges();
// Continue navigation
return false;
return !(await this.promptSaveChanges());
};
private handleBackButtonPress = (): boolean => {
@ -300,8 +301,10 @@ class ConfigScreenComponent extends BaseScreenComponent<ConfigScreenProps, Confi
if (this.hasUnsavedChanges()) {
void (async () => {
await this.promptSaveChanges();
await goBack();
const result = await this.promptSaveChanges();
if (result) {
await goBack();
}
})();
return true;
}

View File

@ -5,7 +5,6 @@ import { View, Text, TextInput } from 'react-native';
import Setting, { AppType } from '@joplin/lib/models/Setting';
import Dropdown from '../../Dropdown';
import { ConfigScreenStyles } from './configScreenStyles';
import Slider from '@react-native-community/slider';
import SettingsToggle from './SettingsToggle';
import FileSystemPathSelector from './FileSystemPathSelector';
import shim from '@joplin/lib/shim';
@ -92,33 +91,31 @@ const SettingComponent: React.FunctionComponent<Props> = props => {
/>
);
} else if (md.type === Setting.TYPE_INT) {
const unitLabel = md.unitLabel ? md.unitLabel(props.value) : props.value;
const minimum = 'minimum' in md ? md.minimum : 0;
const maximum = 'maximum' in md ? md.maximum : 10;
const label = md.label();
const value = props.value?.toString();
const label = md.unitLabel?.toString() !== undefined ? `${md.label()} (${md.unitLabel(md.value)})` : `${md.label()}`;
// Note: Do NOT add the minimumTrackTintColor and maximumTrackTintColor props
// on the Slider as they are buggy and can crash the app on certain devices.
// https://github.com/laurent22/joplin/issues/2733
// https://github.com/react-native-community/react-native-slider/issues/161
return (
<View key={props.settingId} style={styleSheet.settingContainer}>
<Text key="label" style={styleSheet.settingText} nativeID={labelId}>
{label}
</Text>
<View style={{ display: 'flex', flexDirection: 'row', alignItems: 'center', flex: 1 }}>
<Text style={styleSheet.sliderUnits}>{unitLabel}</Text>
<Slider
<View key={props.settingId} style={{ flexDirection: 'column', borderBottomWidth: 1, borderBottomColor: theme.dividerColor }}>
<View key={props.settingId} style={containerStyle}>
<Text key="label" style={styleSheet.settingText}>
{label}
</Text>
<TextInput
keyboardType="numeric"
autoCorrect={false}
autoComplete="off"
selectionColor={theme.textSelectionColor}
keyboardAppearance={theme.keyboardAppearance}
autoCapitalize="none"
key="control"
style={{ flex: 1 }}
step={md.step}
minimumValue={minimum}
maximumValue={maximum}
value={props.value}
onValueChange={newValue => void props.updateSettingValue(props.settingId, newValue)}
accessibilityHint={label}
style={styleSheet.settingControl}
value={value}
onChangeText={newValue => props.updateSettingValue(props.settingId, newValue?.replace(/[^0-9-]/g, ''))}
maxLength={15}
secureTextEntry={!!md.secure}
/>
</View>
{descriptionComp}
</View>
);
} else if (md.type === Setting.TYPE_STRING) {

View File

@ -1450,7 +1450,6 @@ DEPENDENCIES:
- react-native-rsa-native (from `../node_modules/react-native-rsa-native`)
- "react-native-saf-x (from `../node_modules/@joplin/react-native-saf-x`)"
- react-native-safe-area-context (from `../node_modules/react-native-safe-area-context`)
- "react-native-slider (from `../node_modules/@react-native-community/slider`)"
- react-native-sqlite-storage (from `../node_modules/react-native-sqlite-storage`)
- react-native-version-info (from `../node_modules/react-native-version-info`)
- react-native-webview (from `../node_modules/react-native-webview`)
@ -1604,8 +1603,6 @@ EXTERNAL SOURCES:
:path: "../node_modules/@joplin/react-native-saf-x"
react-native-safe-area-context:
:path: "../node_modules/react-native-safe-area-context"
react-native-slider:
:path: "../node_modules/@react-native-community/slider"
react-native-sqlite-storage:
:path: "../node_modules/react-native-sqlite-storage"
react-native-version-info:

View File

@ -32,7 +32,6 @@
"@react-native-community/geolocation": "3.3.0",
"@react-native-community/netinfo": "11.3.2",
"@react-native-community/push-notification-ios": "1.11.0",
"@react-native-community/slider": "4.5.2",
"assert-browserify": "2.0.0",
"buffer": "6.0.3",
"constants-browserify": "1.0.0",

View File

@ -4,7 +4,7 @@ const ObjectUtils = require('../../../ObjectUtils');
const { _ } = require('../../../locale');
import { createSelector } from 'reselect';
import Logger from '@joplin/utils/Logger';
import shim from '../../../shim';
import { type ReactNode } from 'react';
import { type Registry } from '../../../registry';
import settingValidations from '../../../models/settings/settingValidations';
@ -155,7 +155,11 @@ export const saveSettings = async (comp: ConfigScreenComponent) => {
const validationMessage = await settingValidations(savedSettingKeys, comp.state.settings);
if (validationMessage) {
alert(validationMessage);
await shim.showMessageBox(validationMessage, {
type: 'error',
buttons: [_('OK')],
});
return false;
}

View File

@ -810,7 +810,10 @@ class Setting extends BaseModel {
public static formatValue(key: string | SettingItemType, value: any) {
const type = typeof key === 'string' ? this.settingMetadata(key).type : key;
if (type === SettingItemType.Int) return !value ? 0 : Math.floor(Number(value));
if (type === SettingItemType.Int) {
// Retain invalid values as a string, so they can be validated on save without modifying the user input as the user types
return value !== '' && !isNaN(value) ? Math.floor(Number(value)) : value;
}
if (type === SettingItemType.Bool) {
if (typeof value === 'string') {

View File

@ -3,6 +3,7 @@ import shim from '../../shim';
import BaseItem from '../BaseItem';
import Resource from '../Resource';
import Setting from '../Setting';
import { SettingItemType } from './types';
// Should return an error message if there's a problem, and an empty string if not.
// eslint-disable-next-line @typescript-eslint/no-explicit-any -- Old code before rule was applied
@ -37,24 +38,47 @@ const validations: Record<string, ValidationHandler> = {
return '';
},
};
const validateInteger = async (key: string, newValue: string) => {
const md = Setting.settingMetadata(key);
const minimum = 'minimum' in md ? md.minimum : 0;
const maximum = 'maximum' in md ? md.maximum : 10;
const newValueInt = Math.floor(Number(newValue));
if (newValue === '' || isNaN(newValueInt) || newValueInt > maximum || newValueInt < minimum) {
return _('%s must be a valid integer between %s and %s', md.label(), minimum, maximum);
}
return '';
};
// eslint-disable-next-line @typescript-eslint/no-explicit-any -- Old code before rule was applied
const validateSetting = async (settingName: string, oldValue: any, newValue: any) => {
if (oldValue === newValue) return '';
if (!validations[settingName]) return '';
const validateSetting = async (settingName: string, newValues: Record<string, any>) => {
const md = Setting.settingMetadata(settingName);
const oldValue = md.value;
const newValue = newValues[settingName];
// Type based validations
if (md.type === SettingItemType.Int) {
if (oldValue?.toString() === newValue?.toString()) return '';
const message = await validateInteger(settingName, newValue);
if (message !== '') return message;
} else {
if (oldValue === newValue) return '';
}
// Custom validations
if (!validations[settingName]) return '';
return await validations[settingName](oldValue, newValue);
};
// eslint-disable-next-line @typescript-eslint/no-explicit-any -- Old code before rule was applied
export default async (settingKeys: string[], newValues: Record<string, any>) => {
for (const key of settingKeys) {
const oldValue = Setting.value(key);
const newValue = newValues[key];
const message = await validateSetting(key, oldValue, newValue);
return message;
const message = await validateSetting(key, newValues);
if (message !== '') return message;
}
return '';
};

View File

@ -3201,36 +3201,6 @@ From https://github.com/react-native-community/push-notification-ios.
**MIT**:
```
MIT License
Copyright (c) 2020 react-native-community
Permission is hereby granted, free of charge, to any person obtaining a copy
of this software and associated documentation files (the "Software"), to deal
in the Software without restriction, including without limitation the rights
to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
copies of the Software, and to permit persons to whom the Software is
furnished to do so, subject to the following conditions:
The above copyright notice and this permission notice shall be included in all
copies or substantial portions of the Software.
THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
SOFTWARE.%
```
### @react-native-community/slider@4.4.4
From https://github.com/callstack/react-native-slider.
**MIT**:
Copyright: Copyright (c) 2019 react-native-community
See [Appendix B](#appendix-b-the-mit-license) for the full MIT license.

View File

@ -8270,7 +8270,6 @@ __metadata:
"@react-native-community/geolocation": 3.3.0
"@react-native-community/netinfo": 11.3.2
"@react-native-community/push-notification-ios": 1.11.0
"@react-native-community/slider": 4.5.2
"@react-native/babel-preset": 0.74.86
"@react-native/metro-config": 0.74.87
"@sqlite.org/sqlite-wasm": 3.46.0-build2
@ -11063,20 +11062,6 @@ __metadata:
languageName: node
linkType: hard
"@react-native-community/slider@npm:4.4.4":
version: 4.4.4
resolution: "@react-native-community/slider@npm:4.4.4"
checksum: 65d79b72d100aab75e9051315798935a2419202e157f5e35dedb4e2843ccdd93816b553c9a7a41642f5140e5a05e20326a27ef65e9ed9c53efc59d8755a5d91f
languageName: node
linkType: hard
"@react-native-community/slider@patch:@react-native-community/slider@npm%3A4.4.4#./.yarn/patches/@react-native-community-slider-npm-4.4.4-d78e472f48.patch::locator=root%40workspace%3A.":
version: 4.4.4
resolution: "@react-native-community/slider@patch:@react-native-community/slider@npm%3A4.4.4#./.yarn/patches/@react-native-community-slider-npm-4.4.4-d78e472f48.patch::version=4.4.4&hash=1120f2&locator=root%40workspace%3A."
checksum: c4397dd2e914f52e3d9b4058d3cf850e67d99c85a59492835647513af85ba62ba182c5c7655fd35d6f768155d45c0c8b5eb0adaad803165d9fec508b77b19a2b
languageName: node
linkType: hard
"@react-native/assets-registry@npm:0.74.83":
version: 0.74.83
resolution: "@react-native/assets-registry@npm:0.74.83"