Details
Tested correct and incorrect passwords.
Diff Detail
- Repository
- rCOMM Comm
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
I don't think that handling this on submit is a good idea. We should do that earlier by disabling a login button.
That's exactly the way we handle it on native. I thought that we would want to have some consistency between them. We also handle email-like, and not-alphanumeric usernames on submit (on web).
I like your idea, but I think we should go with this for now and create task to rewrite all those cases in the way that they block submit.
I don't think that disabling a button would be hard, but also don't think that prioritizing it before our goals is a good idea. So let's create a task for it and we can land the code from this diff - it is still an improvement comparing to the current version.
| web/account/log-in-form.react.js | ||
|---|---|---|
| 104 ↗ | (On Diff #20621) | It is a different message than on native https://github.com/CommE2E/comm/blob/master/native/account/log-in-panel.react.js#L223-L224 but it is more or less consistent with other messages in this file. It would be more consistent if it was e.g. non-empty passwords only, but I don't think it makes sense to do that @ashoat. Maybe password cannot be empty? |
| 105 ↗ | (On Diff #20621) | In this case it would make more sense to focus the password input. But it doesn't make sense to invest time in this when we're planning to have a different solution, so let's keep it as it is. |
| web/account/log-in-form.react.js | ||
|---|---|---|
| 104 ↗ | (On Diff #20621) | Sounds good to me, I'm okay with either password is empty or password cannot be empty |