Disable form validation for text properties

[hanzi]

fixes #1214
relates to #1324
relates to #1395

Currently, we mark some input fields as type="url", type="tel" and type="email" depending on the property name and sometimes depending on what ical.js expects those fields to contain.

This triggers form validation in some browsers which will then highlight the field as erroneous if the contents don't match the type.

Reasons against this behaviour:

1. Some affected fields such as RELATED or IMPP might genuinely need to contain values that do not match the usual URL pattern. (For RELATED the RFC even explicitely allows arbitrary text values, for IMPP see Skype login is proclaimed to be wrong #1214)
2. Other clients such as Android's Contacts app allow arbitrary values here, which Nextcloud Server happily accepts and which would then be flagged as erroneous in Nextcloud Contacts.
3. Most importantly: While this field is marked red in the UI, we still save the value anyway. So there's a discrepancy between what Contacts actually does and what it displays to the user.

In order to keep specialised keyboards in mobile browsers, I used the inputmode attribute instead.

[codecov]

Codecov Report

Merging #1418 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #1418   +/-   ##
=======================================
  Coverage   62.31%   62.31%           
=======================================
  Files           4        4           
  Lines          69       69           
=======================================
  Hits           43       43           
  Misses         26       26

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6b995f4...ddcf7d1. Read the comment docs.

[skjnldsv]

Thanks for your awesome work!!

You can also just override types in the rfcProps file. I still think that having email, URL.. is an improvement. But for some properties this is indeed an issue :)

[hanzi]

I thought about that as well, but ultimately decided against doing it that way.

I don't think that it's necessarily the app's job to validate user input at this point. Nothing really relies on this data, it's more of a fancy notebook. So if the user wants to enter seemingly invalid data -- why should we stop them?

[skjnldsv]

I don't think that it's necessarily the app's job to validate user input at this point. Nothing really relies on this data, it's more of a fancy notebook. So if the user wants to enter seemingly invalid data -- why should we stop them?

It's still giving proper feedback to the user.
The priority should always to be compliant to the browser's standards as this is what is available and used on all website, we don't create our own check and tooltip implementations for that reason :)

So if the user wants to enter seemingly invalid data -- why should we stop them?

Because then we get corrupted vcards or the icaljs library reject the changes and we get user reporting the app doesn't work ;)

[hanzi]

The thing is that the validation doesn't prevent the saving of any invalid cards. Saving still occurs because we're not actually using the submit handler or even an HTML form to begin with.

Also, for none of the affected fields were there any issues with either ical.js handling the card or the server accepting it during my tests. I agree that it would be nice to have better feedback if there's an error with ical.js/server but the form validation doesn't do that either. It just adds another layer that might (and does) give false positives.

So at the very least, it's misleading UI: A red outline would usually mean 'cannot submit this' to a user, but we're actually submitting it without asking.

And it's bad UX: Form validation should hint at wrong input, when in fact most of the time this will probably highlight data already coming from the server. (Because as I said, other clients like Android or Outlook will let you fill these forms with whatever you want.)

[skjnldsv]

So if the user wants to enter seemingly invalid data -- why should we stop them?

The thing is that the validation doesn't prevent the saving of any invalid cards. Saving still occurs because we're not actually using the submit handler or even an HTML form to begin with.

Then let's leave it like that?
You said why stop them, but this is not stopping, this is a warning. @jancborchardt please decide ;)

[jancborchardt]

And it's bad UX: Form validation should hint at wrong input, when in fact most of the time this will probably highlight data already coming from the server. (Because as I said, other clients like Android or Outlook will let you fill these forms with whatever you want.)

This seems the strongest argument for changing it: Data you already have might be highlighted, and people are used to put everything in from other platforms. :)

Then again, the type of the fields also triggers different keyboards like a number pad for type=tel. This is especially useful on touch devices and mobile.

Is it possible to have the benefit of that without the hassle of unneeded inline validation?

[skjnldsv]

Is it possible to have the benefit of that without the hassle of unneeded inline validation?

I don't think so :(

[hanzi]

Then again, the type of the fields also triggers different keyboards like a number pad for type=tel. This is especially useful on touch devices and mobile.

Is it possible to have the benefit of that without the hassle of unneeded inline validation?

Yeah, and it's already part of this PR: Using the inputmode attribute. Basically the same as using type but without the validation.

[skjnldsv]

Yeah, and it's already part of this PR: Using the inputmode attribute. Basically the same as using type but without the validation.

Then let's get this in!

[skjnldsv]

Then let's get this in!

Hum, actually no.

[hanzi]

Well, support on desktop browsers isn't really that important for on-screen keyboards, is it?
All relevant mobile browsers do support it.

[skjnldsv]

Fair enough.

[jancborchardt]

+1 for the design reasoning