clients
clients copied to clipboard
bitwarden
[PM-5318] fix: allow submitting member-invite form on enter
Type of change
- [x] Bug fix
- [ ] New feature development
- [ ] Tech debt (refactoring, code cleanup, dependency upgrades, etc)
- [ ] Build/deploy pipeline (DevOps)
- [ ] Other
Objective
Currently, when hitting enter when still focussing the emails field when inviting a user to an organization, will try submitting the form.
However, as updatedOn is set to blur for the emails field, and the blur event not being triggered when submitting the form through hitting enter, this results in an error message informing the user that the input is required.
Closes #7156
Code changes
Intercept the enter key, trigger the blur event on the input, and continue calling submit.
Thank you for your contribution! We've added this to our internal Community PR board for review. ID: PM-5318
Marked as draft as it does solve the issue on Chrome, but not on firefox. Looking into this.
Used keydown instead of keyup and event.preventDefault to also fix the behavior on firefox.
Hi @frederikprijck , thanks for checking in. The team will make time to check this and I will keep you posted.
Thanks for getting back to me @eliykat. I did look into that, and I believe it's a bit of an odd PR making this workaround needed.
However, if the original PR can be reconsidered, that would change things.
What https://github.com/bitwarden/clients/pull/5564 does, is:
- It adds a new validator that checks if the limit of free organization users has reached.
However, this validation does allow inviting users that are already part of the organization. This looks awkward and is what is the reason blur was added (or at least how I understand it).
Imagine the following scenario:
- Organization is Free and the limit is 2
- We have two users [email protected] and [email protected]
When I am now trying to invite another member, and I start typing [email protected] and blur away, it wont be invalid. If I would drop the updateOn: blur, it would show the invalid message until the point I end up at the last m and have the email spelled out. We can work around this if needed, but I'd prefer do it differently (see below)
Now, the thing I wonder is ... Why does this have to happen like this? Why does that matter that it should be valid if we exceed the limit with users that are already part of the organization?
I think the above isn't a question I, as a random outside contributor, can answer. But personally, I would not do this through a validator, but instead do the check outside of the form and have a more generic check that lives outside of the validator (e.g. on form initialization) that does not care about the current organization members, but just shows a message that tells the user they have reached their total seat limit for the plan. Or, we could keep the validator without caring about the current members, which should also work without updating on blur.
I am happy to rework #5564 if you believe the above makes sense, making this PR entirly obsolete and avoid the need to dispatch blur events.
However, do know that you will always have this issue once update on blur is introduced again, and enter is used to submit the form (but that may be a problem for another day if we get to rework #5564).
I experimented with this today and it looks like we can just remove onBlur; our component library handles the validation behaviour already. I'd appreciate it if you could test #8009 and let me know if you agree that it fixes the issue.
If you are saying that the validation library ensures no validations are shown up until a certain point (e.g. blur), and not on keystroke, than I am convinced that PR will solve it.
I will try and take some time to see if this also fixes #7156, but I am pretty convinced it will.
#8009 has been validated and merged so I'll close this one. Thanks for your work here @frederikprijck !