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

fix: validation not respecting step attribute in fast-number-field #6617

Closed
saraelsa opened this issue Jan 23, 2023 · 5 comments
Closed

fix: validation not respecting step attribute in fast-number-field #6617

saraelsa opened this issue Jan 23, 2023 · 5 comments
Labels
closed:obsolete No longer valid status:triage New Issue - needs triage

Comments

@saraelsa
Copy link

🐛 Bug Report

When using fast-number-field, the validation mechanism always assumes a step value of 1 regardless of the specified step value. This makes it impossible to use the field for decimal inputs in a form without disabling validation altogether.

💻 Repro or Code Sample

<!DOCTYPE html>
<html>
    <head>
        <title>FastUI Numeric Decimal Demo</title>
        <style>
            .field:invalid::before {
                content: "Value is invalid";
            }
        </style>
    </head>
    <body>
        <fast-number-field class="field" step="0.01" />

        <script type="module" src="https://cdn.jsdelivr.net/npm/@microsoft/fast-components/dist/fast-components.min.js"></script>
    </body>
</html>

🤔 Expected Behavior

Entering the value 1.05 should not produce any validation error.

😯 Current Behavior

Entering the value 1.05 sets the control state to invalid resulting in the text "Value is invalid" displaying. The CSS included in the sample displays this text only when the :invalid pseudo-selector applies to the control.

💁 Possible Solution

It appears that a proxy element is used for validation purposes here. However, none of the validation-affecting attributes such as step are passed to the element.

A mechanism to pass these attributes through to the proxy element, or possibly just using the actual created input as the proxy rather than a separate input element, could ensure that these attributes are respected.

🔦 Context

I'm trying to create a form that takes in a price. This needs to be a value containing up to 2 decimal places. However, currently, it's not possible to accept a price that is not an integer.

🌍 Your Environment

  • OS & Device: Windows 10 on PC
  • Browser: Tested on Microsoft Edge and Mozilla Firefox
  • Version: Latest available on CDN
@saraelsa saraelsa added the status:triage New Issue - needs triage label Jan 23, 2023
@LuohuaRain
Copy link

Yes, I agree. @tomokusaba any ideas?

@tomokusaba
Copy link

I know something is wrong with Validate.
However, I've never done TypeScript before and don't know how to deal with it now.

@tomokusaba
Copy link

number-field.ts

    public validate(): void {
        
        try {
            parseFloat(this.value);
        } catch (e) {
            super.setValidity(
                this.proxy.validity,
                this.proxy.validationMessage,
                this.control
            );
            return;
        }

        const parsedValue = parseFloat(this.valueAsNumber.toString());
        const validValue = isNaN(parsedValue) ? null : parsedValue;
        if (validValue !== null) {
            const step = this.step || 1;
            const max = this.max !== undefined ? this.max : Number.MAX_VALUE;
            const min = this.min !== undefined ? this.min : -Number.MAX_VALUE;
            const modulo = (validValue - min) % step;
            if (modulo !== 0) {
                validValue - modulo + (modulo < step / 2 ? 0 : step);
            }
            if (validValue > max) {
                super.setValidity(
                    this.proxy.validity,
                    this.proxy.validationMessage,
                    this.control
                );
            }
            if (validValue < min) {
                super.setValidity(
                    this.proxy.validity,
                    this.proxy.validationMessage,
                    this.control
                );
            }
        }
    }

@chrisdholt
Copy link
Member

I think we need an explicit version here - the fast-components package has been deprecated for a bit and the latest alpha has fixes for validation. I’d like to be able to pinpoint this and validate. Thanks!

@janechu
Copy link
Collaborator

janechu commented May 29, 2024

Closing due to deprecation of @microsoft/fast-components.

@janechu janechu closed this as completed May 29, 2024
@janechu janechu added the closed:obsolete No longer valid label May 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
closed:obsolete No longer valid status:triage New Issue - needs triage
Projects
None yet
Development

No branches or pull requests

5 participants