Fix ternary condition for default mask values

[Philipp91]

No description provided.

[vercel]

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/mui-org/material-ui-pickers/g0etz8n47
✅ Preview: https://material-ui-pickers-git-fork-philipp91-maskfix.mui-org.now.sh

[cypress]

---

Test summary

77 • 0 • 1 • 0

---

Run details

Project	material-ui-pickers
Status	Passed
Commit	8f7ceb5
Started	May 25, 2020 11:32 AM
Ended	May 25, 2020 11:33 AM
Duration	01:40 💡
OS	Linux Debian - 9.11
Browser	Chrome 78

View run in Cypress Dashboard ➡️

---

This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard

[oliviertassinari]

Could you add a regression test case with the enzyme mount API?

[Philipp91]

I'm not familiar with that.

[dmtrKovalenko]

Holey moley, that javascript

[oliviertassinari]

~I couldn't find how to reproduce this issue in the CI. ~

[oliviertassinari]

Actually, I can still reproduce the issue with the above change. It seems that it doesn't solve the problem.

[Philipp91]

You're right. I attempted to fix the original issue by adding mask="__.__.____ __:__" to my picker, which matches my locale. Due to a bug that this PR hopefully fixes (though I haven't even tested that), the mask prop that I supplied manually was ignored, so the error message still appears.

However, this doesn't fix the underlying issue tracked in #1776, which is that the mask should be correct automatically. My application happens to be single-locale, so I can just hard-code the correct mask, but not every application can do so, and it shouldn't be necessary. So to really fix #1776, the hard-coded masks that match the American date format need to be replaced with something more dynamic.

I have removed the "fixes #1776" claim from this PR. I still think it fixes a bug though.

[oliviertassinari]

@Philipp91 Awesome, thanks for the extra context! I have tried to add a test case but without any luck. It seems that the mask is ignored. I don't understand why.

[vercel]

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/mui-org/material-ui-pickers/r36nbskxx
✅ Preview: https://material-ui-pickers-git-fork-philipp91-maskfix.mui-org.now.sh

[oliviertassinari]

@Philipp91 I'm sorry. I'm making no progress on this one. I'm giving up. A few observations:

1. /demo/datetime-picker raises the warning mentioned in Mas warning on console The mask "__/__/____ __:__ _M" you passed is not valid for the format used P p. Falling down to uncontrolled not-masked input.  #1776.
2. I had forgotten how testing with enzyme was frustrating compared to testing the DOM. I couldn't figure out how to fire a change event on the input and assert. This fails, god 😆. @dmtrKovalenko I can't want for moving away from it 🙏

      component.find('input').simulate('change', {
        target: {
          value: '12',
        },
      });
      expect((component.find('input').getDOMNode() as HTMLInputElement).value).toBe('12');

3. I couldn't observe the fixed behavior by the pull request. @Philipp91 Do you have a reproduction?

[Philipp91]

To reproduce and verify that this PR works, even if it doesn't fully resolve the issue:

1. Patch Philipp91@f42bd3c. (It deletes a bunch of example code to ensure that there's only one picker on the page to worry about.)
2. Open http://localhost:3001/demo/datetime-picker
3. Note how the patch changes the locale to de-DE. The error message appears saying that the en-US mask hard-coded in the library (namely "__/__/____ __:__ _M") is invalid. Note how the reproduction code attempts to fix that by setting mask="__.__.____ __:__", which would match the de-DE format. (Even if you remove that fix attempt, you'll see the warning.)
4. Patch this PR, then the warnings goes away. It still doesn't work without the explicit mask="__.__.____ __:__" (which it should), but the PR makes it a bit less broken.

I've posted a proposal for the real fix of the underlying issue. That real fix likely won't make the broken ternary expression go away, so it's worth fixing it anyway.

[oliviertassinari]

@Philipp91 Awesome, thanks for the diff. I could figure a regression test out :).

updated

updated

[oliviertassinari]

Rebased to fix the conflicts

[oliviertassinari]

I'm excited about the new test 🎉 :) @Philipp91 Thanks so much for the help. We wouldn't have made it without you.