julkascript
commented
4 months ago It looks very good, feels nice, good job as always!
I noticed the case where you bypass the frontend validation with a wrong email and you get 400 from the backend but it's not displayed as error message. It would be a good idea to handle this. We don't need to match Django's validations but it would be enough just to display the toast message with the reason for the failure. This would apply also if you manually make the backend return other responses like 4XX or 5XX to simulate the service being down. Always a good idea to keep the user informed.
RyotaMitaraiWeb
This PR tackles #95, with two caveats:
The reason for this is that there is no point in sending an invalid registration to the server. The validations also apply in the login page for similar reasons. The submit button is disabled when any of the fields are invalid or when at least one field has not been changed at all (inputting anything in a field will cause a debounced status update, which makes the field "dirty").
The only validation that is not performed client-side is for whether the username is already in use, as there is no way to check that before sending the request (in the case of the register page).
If the username is already in use, the error is placed in the username field, though a simple input change will clear it off.
Note 1: the email pattern used on the front end likely doesn't completely match the one that comes straight out of Django. The only reason I haven't tried to make them consistent is because it seems like Django performs multiple regex tests on the email and this would require some tweaking on my end. Whether this is worth doing is a different question (since all of the common email domains I could think of are valid in both cases), but in case it is, consider my current implementation more of a partial POC at this stage.
Note 2: this PR features a small change to the debounce hook, which makes it possible to pass arguments to the generated function. The reason for this is that I wanted to use one hook for multiple status changes (in regards to whether a particular field has been changed or not) out of laziness, but this ended up causing issues in the cases where a user fills multiple fields very quickly (in which case it did not update all fields' status), so I had to abandon this idea. So basically, there's a small change in the debounce hook that ended up totally irrelevant to the issue at hand.
Note 3: I haven't applied any of this in the profile page, since I want to make sure that my approach is fine before I touch that page (in case it is, I will probably tackle this in a separate PR).