[TextField] Implemented masking by lambertwang-zz · Pull Request #3783 · microsoft/fluentui

lambertwang-zz · 2018-01-24T01:35:58Z

Pull request checklist

Addresses an existing issue: Fixes [TextField] It's would be nice if we support mask attribute for TextField #3769
Include a change request file using $ npm run change

Description of changes

Implemented input masking (similar to https://www.npmjs.com/package/react-input-mask)

Note: the MaskedTextField is its own component. The original TextField component should be untouched.

joschect · 2018-01-24T01:53:54Z

+    return outputMask;
+  }
+
+  private _extractValueFromMask(maskedValue: string, element: HTMLInputElement | HTMLTextAreaElement): string {


I don't think you use element any where in this method

joschect · 2018-01-24T01:59:20Z

+    // Check for backspace
+    if (key == 8) {
+      if (this.props.mask && this.state.maskCursorPosition && this.state.value) {
+        if ((event.target as HTMLInputElement).selectionStart >= this.state.maskCursorPosition) {


I can select multiple characters and still satisfy this condition, but the value will only be set as 1 less than my current value length.

If you select multiple characters and backspace them, the inputChanged event will detect that you deleted a bunch of characters. Like if you ctrl+a and backspace, it will clear any input the user has made and display the empty mask.

joschect · 2018-01-24T18:40:49Z

+            valueIndex < maskedValue.length &&
+            maskedValue.charAt(valueIndex) != mask.charAt(maskIndex)) {
+            skippedChars++;
+            valueIndex++;


Won't it be possible for the valueIndex be 1 higher than you expect since it will increment once this while loop exits? At least if you have text that's something like a_a_a

That is intentional.

…into magellan/textFieldMasking

cschleiden · 2018-01-27T05:00:06Z

 import * as React from 'react';
 /* tslint:enable:no-unused-variable */
-import { TextField } from 'office-ui-fabric-react/lib/TextField';
+import { DefaultTextField } from 'office-ui-fabric-react/lib/TextField';


Why do you change all of the examples to use DefaultTextField instead of TextField? That doesn't seem right

For the same reason all of the Button examples were changed to use DefaultButton instead of Button.

What about compat, when I use TextField today, will I have to change to DefaultTextfield with this change?

No, just as with button, the TextField component still exists, it will just warn you for using a deprecated component.

@lambertwang I don't really think that's a good idea.

…into magellan/textFieldMasking

dzearing · 2018-02-07T06:22:59Z

              <tr>
                <td>
-                  <TextField
+                  <DefaultTextField


Why are you renaming everything that was TextField to DefaultTextField? That feels counterintuitive.

jordandrako · 2018-02-20T21:45:20Z

Any updates on this @lambertwang?

…t into magellan/textFieldMasking

dzearing · 2018-02-21T23:36:45Z

  inputProperties,
  textAreaProperties
 } from '../../Utilities';
+import { AnimationClassNames } from '../../Styling';


dzearing · 2018-02-21T23:40:08Z

        />
+        <MaskedTextField
+          label='With input mask'
+          mask='Phone Number: (999) 999 - 9999'


I don't understand how this example works. I will try to get a demo from you.

dzearing

This is a little confusing, can you demo for me?

…c-react into magellan/textFieldMasking # Conflicts: # packages/office-ui-fabric-react/src/components/ContextualMenu/examples/ContextualMenu.Directional.Example.tsx

…t into magellan/textFieldMasking

…fice-ui-fabric-react into magellan/textFieldMasking

dzearing · 2018-03-27T00:27:07Z

@@ -0,0 +1,325 @@
+import * as React from 'react';


This is missing tests. Needs a bunch of enzyme tests. And probably a snapshot test or two.

dzearing · 2018-03-27T00:27:19Z

+  "changes": [
+    {
+      "packageName": "office-ui-fabric-react",
+      "comment": "TextField: Inmplemented input masking.",


dzearing

needs test.

…into magellan/textFieldMasking

lambertwang-zz · 2018-03-28T17:33:25Z

@dzearing @cschleiden Can I get another review? I've made some changes and added enzyme tests for the component.

Last response 82 days ago.

…into magellan/textFieldMasking

cliffkoh

As Travis CI indicated, your change has a lot of null check failures. Please update your submission to resolve the issues.. I will take a look at your unit tests though.

cliffkoh · 2018-04-19T02:02:57Z

@lambertwang FTFY: lambertwang-zz#1

…king Fix build break issues with your PR

cliffkoh

@lambertwang You now have a bunch of unused vars errors due to the recent change to tslint :)

The tests look fine. I did not look very closely at other changes besides the test since other reviewers have looked at it so approving for now.

…into magellan/textFieldMasking

* master: DetailsRow: Flexshrink fix (microsoft#4622) [TextField] Implemented masking (microsoft#3783) Applying package updates. ComboBox: Add any event as additional parameter to onChanged callback for saving pending changes (microsoft#4594) Remove usage of Number.NaN (microsoft#4615)

lambertwang-zz added 3 commits January 23, 2018 17:33

TextField: Implemented masking

251e839

Changed example

71cea3f

Change file

4bf0059

lambertwang-zz requested a review from joschect January 24, 2018 01:36

joschect reviewed Jan 24, 2018

View reviewed changes

lambertwang-zz added 3 commits January 24, 2018 14:51

Moved functions to util. Added tests

ab1c2c7

Added selection deletion tests

cfd5e0a

Refactored textfield into separate components

c61c77c

lambertwang-zz requested review from cschleiden, johannao76 and lorejoh12 as code owners January 26, 2018 22:37

Merge branch 'master' of github.com:OfficeDev/office-ui-fabric-react …

b932927

…into magellan/textFieldMasking

lynamemi mentioned this pull request Jan 26, 2018

[TextField] It's would be nice if we support mask attribute for TextField #3769

Closed

cschleiden previously requested changes Jan 27, 2018

View reviewed changes

Merge branch 'master' of github.com:OfficeDev/office-ui-fabric-react …

e2462f0

…into magellan/textFieldMasking

dzearing reviewed Feb 7, 2018

View reviewed changes

manishgarg1 added the enhancement label Feb 15, 2018

manishgarg1 assigned dzearing Feb 15, 2018

lambertwang-zz added 4 commits February 21, 2018 11:12

Merge branch 'master' of github.com:lambertwang/office-ui-fabric-reac…

257ea8a

…t into magellan/textFieldMasking

Deleted base and default textfields

86e3260

Revert additional changes

76c3d1b

Fixed refs

322141d

dzearing reviewed Feb 21, 2018

View reviewed changes

dzearing requested changes Feb 21, 2018

View reviewed changes

dzearing added 2 commits March 10, 2018 15:33

Update magellan-textFieldMasking_2018-01-24-01-35.json

794d948

michaelangotti mentioned this pull request Mar 22, 2018

TextField as Number #4328

Closed

Merge branch 'master' of github.com:lambertwang/office-ui-fabric-reac…

2ac854f

…t into magellan/textFieldMasking

Merge branch 'magellan/textFieldMasking' of github.com:lambertwang/of…

17cff11

…fice-ui-fabric-react into magellan/textFieldMasking

dzearing reviewed Mar 27, 2018

View reviewed changes

dzearing requested changes Mar 27, 2018

View reviewed changes

lambertwang-zz added 6 commits March 27, 2018 11:28

Added enzyme and snapshot tests for the component

b66bdb9

Fixed typo

3ea706d

Added character overflow test

218637f

Undo changes to textfield

fc57283

Merge branch 'master' of github.com:OfficeDev/office-ui-fabric-react …

ed138ae

…into magellan/textFieldMasking

Fixed linting error

f47a481

lambertwang added 3 commits March 28, 2018 10:39

Update TextField.Basic.Example.tsx

1cfbdf7

Added comments to inputMask.ts

7060aee

Removed trailing whitespace

5eb7615

Merge branch 'master' of github.com:OfficeDev/office-ui-fabric-react …

7776a30

…into magellan/textFieldMasking

lambertwang-zz requested a review from cliffkoh April 18, 2018 21:25

cliffkoh suggested changes Apr 19, 2018

View reviewed changes

cliffkoh added 2 commits April 18, 2018 18:52

Fix build errors for Lambert

ade5b57

Update jest snapshot

41353cd

Merge pull request #1 from cliffkoh/lambertwang-magellan/textFieldMas…

ef1e721

…king Fix build break issues with your PR

cliffkoh approved these changes Apr 19, 2018

View reviewed changes

lambertwang-zz added 2 commits April 19, 2018 14:13

Merge branch 'master' of github.com:OfficeDev/office-ui-fabric-react …

ed03b12

…into magellan/textFieldMasking

Fix build errors

5087641

dzearing approved these changes Apr 19, 2018

View reviewed changes

lambertwang-zz merged commit bd2e7b1 into microsoft:master Apr 19, 2018

microsoft locked as resolved and limited conversation to collaborators Aug 31, 2019

Uh oh!

Conversation

lambertwang-zz commented Jan 24, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull request checklist

Description of changes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dzearing Feb 7, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jordandrako commented Feb 20, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dzearing left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dzearing left a comment

Choose a reason for hiding this comment

Uh oh!

lambertwang-zz commented Mar 28, 2018

Uh oh!

cliffkoh left a comment

Choose a reason for hiding this comment

Uh oh!

cliffkoh commented Apr 19, 2018

Uh oh!

cliffkoh left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

lambertwang-zz commented Jan 24, 2018 •

edited

Loading

dzearing Feb 7, 2018 •

edited

Loading