Mobile: Resolves #10883: Remove slider component module and replace integer settings with new validated component #11822
+166
−165
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
laurent22
reviewed
Feb 27, 2025
| interface Props { | ||
| settingId: string; | ||
| // eslint-disable-next-line @typescript-eslint/no-explicit-any -- Old code before rule was applied | ||
| value: any; |
There was a problem hiding this comment.
Wouldn't the value always be a string? (or a number?)
There was a problem hiding this comment.
You're right. I've now updated it to a number type
86 tasks
|
Thanks for fixing this @mrjo118, that looks good to me. Just running CI again and waiting to pass (probably was an unrelated random failure) |
|
It was a variable no longer in use due to the change being combined with PR #11821 . I've pushed up a commit now which should fix that |
|
All good now |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is an alternative solution to PR #11069 which minimises the impact, so that validation is isolated to a new ValidatedIntegerInput component. This was proposed by @personalizedrefrigerator in order to reduce the scope of the change, making it easier to review and test.
On mobile, a react native slider component is used where integer values are entered as a configuration setting. This is not user friendly where the value is a large number. The desktop app instead uses a text input with the number type, which disallows restricted characters to be entered and also has increment and decrement buttons which appear when hovering over (which are capped at the min and max values for the setting). In order to improve usability on mobile, I have removed the react-native-community/slider component entirely from the project, and implemented a custom TextInput component to replace them in the mobile app, which utilises validation hints for invalid and out of bound values.
Note that in the case where the user attempts to save an invalid value, or leave the screen with an invalid value present, the value will not save / be reverted to the original saved value. Also note that if the value of an integer configuration setting is changed to invalid values only, a new change to enable the save button will not occur until a valid value which is different to the stored value has been entered.
The attached video shows the new behaviour:
https://github.com/user-attachments/assets/eb839075-fdd7-4baf-8efa-35533adfb321
This will resolve #10883