Fixes #2725: Bugform validation improvements

[ksy36]

r? @miketaylr

[miketaylr]

For the future, it's kinda nice to read individual commits to understand the changes, rather than a giant one without a lot of context. If you prefer squashing after, that's fine too.

What's the plan for indicating required form elements to the user? It seems like we've removed that here except for the one that says mandatory.

[ksy36]

Yeah, sorry :) I had individual commits in v1 branch, but then after we refactored the form here, decided to move these validation changes to v2 branch and base them on the refactored version. So they all landed in one commit..

For the required fields there were 2 more missing (mandatory) , added them.

[ksy36]

Actually I've just noticed a small issue, will be fixing it shortly.

[miketaylr]

Yeah, sorry :)

No issue! Just makes it a bit easier to review in chunks.

[ksy36]

Ok the last issue is fixed now. I made it so the errors don't show up on page refresh or when coming from the report site issue extension, so a user will see them only when clicking on submit.

@miketaylr could you please review the commit?

[miketaylr]

Awesome. Thanks!

[miketaylr]

@karlcow can we push this branch to staging to do a quick sanity check the form still behaves as we expect. It LGTM locally.

[miketaylr]

(we can merge now, or wait until we check it out on staging, no strong preference here)

[karlcow]

@ksy36 @miketaylr this branch is deployed on staging.

[miketaylr]

@ksy36 @miketaylr this branch is deployed on staging.

Thanks Karl. I'll poke around before merging.

[miketaylr]

@ksy36 I think I discovered a bug:

https://staging.webcompat.com/issues/1882

STR:

open the bug form on staging.webcompat.com/issues/new
put in a URL
select a problem type
write some summary
click back to the URL input
select all the text (cmd+a, on mac, for example)
delete all the text (cmd+x, on mac, for example)
without changing focus, use the mouse to click submit.

Expected: the form should not submit
Actual: the form submits, with a None URL.

I checked the STR on production, and if you delete all the text, we run validation. So this would technically be a regression (probably a super-corner case).

Do you want to try to fix here? Or in a follow up bug?

[ksy36]

@miketaylr weird, I'm not able to reproduce - the button becomes slightly grey and disabled once I press cmd+x and the text clears out. When I click on the button, the url error shows up as expected.
Am I missing something?

[miketaylr]

I'm not 100% sure. But it's pretty edge-casey, so let's merge here and I'll file a follow up if I can reproduce in a new profile.