usds / benefits-enrollment-prototype

https://usds.github.io/benefits-enrollment-prototype/
31 stars 13 forks source link

IE9 - Screens will not advance #1

Closed jaredcunha closed 7 years ago

jaredcunha commented 7 years ago

This isn't an error that will have any impact on the rails site, but it would impact testing on IE9. We can circumvent this by requiring testers to use a modern browser. Just not sure if it's worth putting effort into this point.

cc @mollieru

mollieru commented 7 years ago

Sorry -- to be clear, the demo app doesn't advance in IE9?

jaredcunha commented 7 years ago

Yeah, IE9 doesn't support checkValidity which I'm using for validation. Hence the bugging out.

mollieru commented 7 years ago

Hmmmm. seems like there's a decent chance there are people who will be using IE9, and it's a thing we generally support. Is it a lot of work to fix? Maybe something @raquelromano can poke around with?

jaredcunha commented 7 years ago

@raquelromano just tinkering around and i think i got this!

raquelromano commented 7 years ago

Guess we duplicated some work here. I went ahead and committed my changes though (same effect, just pulled into a single function in session-storage.js)

raquelromano commented 7 years ago

Oh. Now that I see where certain values are assumed to be defined in the JavaScript, IE9 users may still encounter errors when validation is skipped.

Back to what Jared says above about whether or not it's worth fixing since we will do this completely differently in the rails app.

But one terrible hack for the moment is to find all the places where a form field is required to be defined and have it default to some value if the user doesn't supply it. I'm not sure that's any better though.

jaredcunha commented 7 years ago

@raquelromano, ha, sorry about that! I didn't see that you self-assigned until I already pushed the work up. It was a quick fix.

Is worth making a note that validations are only supported in modern browsers in the demo only?

To your last point, was that in reference to the demo or error handling altogether?

mollieru commented 7 years ago

This has been addressed yea?

jaredcunha commented 7 years ago

@mollieru - We've resolved the issue in terms of getting the demo form to advance in IE9.

The demo uses HTML5 browser validation, which unfortunately does not work in IE9. So it possible to advance the form, and errors or missing fields could have an impact downfield.

How we approach validation in a functioning application is worthy of another discussion, in perhaps a separate issue. My approach would be layered as follows:

  1. Use HTML5 validation for modern browsers. This is the easiest implementation of validation since it requires relatively no JS, just a little to prevent Safari from submission. However, this validation functions in Chrome with no JS whatsoever. It's available to 96.91% of users (global stat), optimizes performance, and includes built-in accessibility features. The alternative to would be to use a JS library that isn't predicated on checkValidity. However, I think in doing that, while it's possible to support IE9 and below on the client side, you lose any benefits of browsers improving their native implementation.
  2. Server-side validation. This is required, as it is primary means of preventing incorrect/missing data from going into the database. Client-side validation can fail because of errors in production or blocking third-party scripts, people can manipulate pattern criteria in the web inspector (note, this would be highly unlikely, though theoretically it can be done). The reason we don't want to rely solely on server-side validation is that client-side validation is much faster and friendlier on our servers.

Of course, the approach I've outlined will not resolve the issue of validation of IE9 in the demo, but it would resolve the issue of validation IE9 in the actual application. I'm open to other ideas, but thems my two cents.

raquelromano commented 7 years ago

Yep, we'll do both client- and server-side validaiton in the web application and use a polyfill on the client: https://www.html5rocks.com/en/tutorials/forms/constraintvalidation/#toc-polyfilling

We could do that here as well for anyone browsing the demo on those other browsers since there are still issues if fields are left blank. I'd say it's a nice to have for this demo.

jaredcunha commented 7 years ago

Ah yeah - shims!

mollieru commented 7 years ago

Will be resolved in https://github.com/usds/benefits-enrollment-prototype/issues/58