Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] Modal verification of number type incorrectly flags decimals #186

Closed
andrew-m-j opened this issue Dec 29, 2023 · 15 comments
Closed

[BUG] Modal verification of number type incorrectly flags decimals #186

andrew-m-j opened this issue Dec 29, 2023 · 15 comments
Assignees
Labels
bug Something isn't working obsidian-limitation

Comments

@andrew-m-j
Copy link

Describe the bug
When entering data in a form for a number type item, the entry suggests a validation error for decimal entry numbers by highlighting the entry box in red. However, the data seems to be parsed correctly to the results still, with the decimal number stored.

To Reproduce
Steps to reproduce the bug:

  1. Enter a decimal number on a number type field (e.g, "123.456").
  2. Entry box is highlighted in red, suggesting an error.
  3. Submitting the form and accessing the results indicate that the decimal is stored correctly.

Expected behavior

Form validation of the number type should not flag a decimal as an error. The text entry already seems to prevent characters from being entered. Other fringe cases that might need to be handled are if there are two decimals entered, which is an error (e.g., "123.456.789"), or for comma entry instead of a decimal as in some countries.

@andrew-m-j andrew-m-j added bug Something isn't working enhancement New feature or request labels Dec 29, 2023
@fetwar
Copy link
Contributor

fetwar commented Apr 12, 2024

+1

This bug is preventing decimals from being input for me in the same manner. I am able to reproduce the bug by following the instructions from @andrew-m-j. With the same result

Having a very brief look at the code though... I can't actually find where the validators for Number are.

Do you mind pointing me in the right direction for this one and I will attempt a fix @danielo515

@danielo515
Copy link
Owner

There is no number validation, that is why you don't see any @fetwar
If the form were validating the value, and you input an invalid value, you will not be able to submit the form, but, if you try, you will see you will be able to submit the form without problem.
My guess is that this is either a problem with obsidian itself, or a problem in a theme that is incorrectly applying the wrong styles.
I just tested any random obsidian input, changed it to be of type number and then it exhibits the same behaviour. So this is either a but in Obsidian core or a theme.

@danielo515 danielo515 added obsidian-limitation and removed enhancement New feature or request labels Apr 12, 2024
@fetwar
Copy link
Contributor

fetwar commented Apr 12, 2024

Hmmm, sounds like it might be a css style that is being apply for generic field "Number".

Could this perhaps be changed to a custom field type?

I will have a look at the CSS styles being applied and see if we find anything

@fetwar
Copy link
Contributor

fetwar commented Apr 12, 2024

CSS styles applied on app.css:

input[type='number'] {
  -webkit-app-region: no-drag;
  background: var(--background-modifier-form-field);
  border: var(--input-border-width) solid var(--background-modifier-border);
  color: var(--text-normal);
  font-family: inherit;
  padding: var(--size-4-1) var(--size-4-2);
  font-size: var(--font-ui-small);
  border-radius: var(--input-radius);
  outline: none;
}

These are consistent whether in error state or not, the var values themselves are changing - No they aren't lol, new styles are being applied that I apparently missed

@fetwar
Copy link
Contributor

fetwar commented Apr 12, 2024

The color and border-color attributes are being applied by the css classes here:

.modal-form input:invalid,
.modal-form .invalid {
border-color: var(--text-error);
color: var(--text-error);
}

Which I am guessing means Obsidian is applying the "invalid" attribute as you said.

What are your thoughts on the idea of a non-number input or only showing the error for this css class if modal-form-invalid is applied to specifically number?

Those are both workarounds though

@danielo515
Copy link
Owner

What are your thoughts on the idea of a non-number input or only showing the error for this css class if modal-form-invalid is applied to specifically number?

The first one is a no-go. I started this project with the idea of using the standard Obsidian inputs as much as possible, and I also like to stick to standard html inputs for accessibility reasons.
The second one, may be a viable workaround, but I would like to see what the obsidian team has to say if we report this error before we do anything on our side

@danielo515
Copy link
Owner

Interestingly, this is standard HTML behaviour:
https://developer.mozilla.org/en-US/docs/Web/HTML/Element/input/number#allowing_decimal_values
So the workaround is simple, add a step of 0.001

@fetwar
Copy link
Contributor

fetwar commented Apr 12, 2024

I 100% agree, it seems odd that standard "number" following HTML standards is in an "invalid" state. Integer sure, number definitely not

@fetwar
Copy link
Contributor

fetwar commented Apr 12, 2024

Sounds like a great workaround, I appreciate your investigations on this

@fetwar
Copy link
Contributor

fetwar commented Apr 14, 2024

I think a step of any would be a more reasonable step to not unnecessarily constrain users who may want smaller decimal granularity.

From https://developer.mozilla.org/en-US/docs/Web/HTML/Element/input/number#step

A string value of any means that no stepping is implied, and any value is allowed (barring other constraints, such as min and max).

@danielo515
Copy link
Owner

danielo515 commented Apr 15, 2024 via email

@fetwar
Copy link
Contributor

fetwar commented Apr 16, 2024

The wording around min and max constraints is a bit unclear, but validation of other constraints such as min, max and required are still applied.

I have created this codepen for testing the validation, and I can't see any issue with the form input working in this way

@fetwar
Copy link
Contributor

fetwar commented Apr 16, 2024

PR opened

@danielo515
Copy link
Owner

Thank you for the testing @fetwar ! that was very useful! Will check the PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working obsidian-limitation
Projects
None yet
Development

No branches or pull requests

3 participants