microsoft / fluentui

Fluent UI web represents a collection of utilities, React components, and web components for building web applications.
https://react.fluentui.dev
Other
18.48k stars 2.73k forks source link

MaskedTextField: value prop updates not setting new displayValue #5879

Closed ParadimeWeb closed 4 years ago

ParadimeWeb commented 6 years ago

Bug Report

Priorities and help requested (not applicable if asking question):

Are you willing to submit a PR to fix? Yes

Requested priority: Blocking

Products/sites affected:

Describe the issue:

When calling setState in an event like a button onClick event, the MaskedTextField displayValue is not set.

Actual behavior:

After setState, the display value of the MaskedTextField remains the same.

Expected behavior:

After setState, the display value of the MaskedTextField changes to the new value.

If applicable, please provide a codepen repro:

https://codepen.io/paradimeweb/pen/PBXWwz

JasonGore commented 6 years ago

I started looking at this and it looks like only the initialize value is unmasked while onChange and all future controlled value props contain the masked version of the value.

It looks like we'd have to refactor internals quite a bit to deal with subsequent unmasked value prop changes as there are no existing provisions for easily resetting cursor, mask data, etc.

@lambertwang I was hoping you could help with insight on design intention here. Do you know if MaskedTextField is purposely designed to only take initial value and ignore further updates to value?

lambertwang-zz commented 6 years ago

MaskedTextField was not designed with that purpose in mind. It is a bug.

JasonGore commented 6 years ago

I don't see any easy fix for this. Since it was not initially designed for this use case and will require some refactoring to support, I'll put this in our backlog.

ghost commented 5 years ago

Since this was urgent and a blocking bug for me I managed to fix it after forking the repo , here are the changes : https://github.com/rachidaoudi/office-ui-fabric-react/commit/e1a6b4b0e144fe5bd5abe037173d0646e47ad216 I couldn't make a pull request since I didn't read through the guidelines but I hope this comes in handy for anyonelse that needed this fix for their project.

ecraig12345 commented 5 years ago

@rachidaoudi Thanks, that could definitely be helpful!

ajain17 commented 5 years ago

Our team is also blocked on this, any ETA on when we can expect the update?

darrenmart commented 5 years ago

Have to add my two cents that this issue makes MaskedTextField essentially useless until it's fixed to update the displayed value when the underlying state changes. Having to replace it with TextField + RegEx isn't ideal as it defeats much of the purpose.

ajain17 commented 5 years ago

I was able to use the workaround listed above and unblock temporarily but its not ideal and makes the component useless out of the box.

ghost commented 5 years ago

As a follow up, turns out that the maskedfield suffers from other bugs as well, so in order to solve the issue once and for all for our team, I decided to make my own out of the Textfield component:

The mask has the same syntax as the old one.

import React from 'react';
import IMask from 'imask';
import { TextField } from 'office-ui-fabric-react';

export default class Maskfield extends React.Component {
    parent = null;
    input = null;
    mask = null;

    _onBlur = () => {
        this.mask.updateControl();
    }
    _onClick = () => {
        this.mask.updateControl();
    }

    componentDidMount() {
        const { mask } = this.props;
        const element = this.parent.querySelector('input');
        const maskOptions = {
            mask,
            definitions: {
                "a": /[a-z ]/,
                "A": /[A-Z ]/,
                "9": /[0-9]/,
                "O": /[A-Z0-9]/,
                "o": /[a-z0-9]/,
                "i": /[a-zA-Z0-9]/,
                "*": /[a-zA-Z0-9 ]/,
                "n": /[a-zA-Z0-9-]/,
                "é": /[a-zA-Zéèàùç ]/,
            }
        };
        this.mask = new IMask(element, maskOptions);
    }

    render() {
        const {  ...other } = this.props;

        return (
            <div ref={e => this.parent = e}>
                <TextField
                    {...other}
                    componentRef={e => this.input = e}
                    onBlur={this._onBlur}
                    onClick={this._onClick}
                />
            </div>
        )
    }
}
jrop commented 5 years ago

This seems similar enough to the issue I am facing: I am attempting to use a MaskedTextField as a controlled component, but updates to value="..." are not updating the displayed value.

khmakoto commented 4 years ago

Hi @ParadimeWeb, sorry for the long time it took to respond to this issue! It seems like the issue has been fixed from what I can see in your codepen. Could you confirm if this is the case? Thanks!

ParadimeWeb commented 4 years ago

Hi @khmakoto, yes it looks like they fixed it. Thanks for your help.