[Fix] 20 Autocomplete Bug (alternative solution) #155
Merged
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.
During my review of PR #149 (
[Fix] 20 Autocomplete Bug
) I tried to understand the root cause of the problem and finally found it.Here is the problem description:
10min
works.1 s
.The root cause of this behavior is the following:
1 s
. When a user types e.g.10 min
there will be internal exceptions thrown for every keystroke until10 min
is entered completely. Whenever an exception is thrown (because partial values like10 mi
are invalid), the return value isnew Period(default)
which equals to1 s
as shown here:nexus/src/Nexus.UI/ViewModels/SettingsViewModel.cs
Line 77 in 98ea2f0
1 s
.10 min
and then tries to enter5 s
, this immediately fails because - as described above - partial values like5
are invalid and cause exceptions to be thrown. This in turn leads to a return value ofnew Period(default)
which in turn equals to1 s
, so now the internal value does change and the input is immediately replaced with1 s
and the the user cannot continue entering the5 s
value.So the obvious solution is to not return
default
ornew Period(default)
but the current value instead until the new input is valid. The current value is not directly accessible in the converter but agetCurrentValue
function can be passed into the converter's constructor to work around this problem as I did in the first commit.