Skip to content
This repository has been archived by the owner on Sep 11, 2024. It is now read-only.

Ignore whitespace in email address check #5623

Closed
wants to merge 3 commits into from

Conversation

dennisroczek
Copy link

@dennisroczek dennisroczek commented Feb 6, 2021

for the case you copy & paste the email address you misses whitespaces. A trim is already integrated in the RegistrationForm.


Here's what your changelog entry will look like:

✨ Features

for the case you copy & paste the email address you misses whitespaces
@SimonBrandner
Copy link
Contributor

@dennisroczek, please include a sign off as described here

dennisroczek and others added 2 commits February 6, 2021 14:11
for the case you copy & paste the email address you misses whitespaces

Signed-off-by: dennisroczek <dennisroczek@libreoofice.org>
…eact-sdk into email-trim

Signed-off-by: Dennis Roczek <dennisroczek@libreoffice.org>
@dennisroczek
Copy link
Author

@dennisroczek, please include a sign off as described here

err, I always miss the -s part :-/

@jryans jryans changed the title ignore whitespace Ignore whitespace in email address check Feb 8, 2021
Copy link
Collaborator

@jryans jryans left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, I don't think we can accept as-is... There are various other components that call this same function which do not trim, so they would then have a confused idea of validity.

If you'd like to move forward here, I suggest visiting each component and checking how they should handle spaces.

@MadLittleMods MadLittleMods added Z-Community-PR Issue is solved by a community member's PR T-Enhancement New features, changes in functionality, performance boosts, user-facing improvements labels Jun 2, 2022
@turt2live
Copy link
Member

@dennisroczek apologies for the delayed reply here - are you able to take a look at this still?

@ankur12-1610
Copy link
Contributor

@dennisroczek apologies for the delayed reply here - are you able to take a look at this still?

Can I get the issue that we're solving coz the issue isn't linked, I tried to find it but I was not able to do so?

@dennisroczek
Copy link
Author

There was no bug report. I found the bug myself and simply tried to fix it. Sadly, haven't the time at all to do anything.

Feel free to overtake.

@ankur12-1610
Copy link
Contributor

There was no bug report. I found the bug myself and simply tried to fix it. Sadly, haven't the time at all to do anything.

Feel free to overtake.

Is it possible for you to add a video or screenshot or something?

@turt2live
Copy link
Member

I think the testing steps for this would be to try to register an account with email, adding spaces to the end.

@ankur12-1610
Copy link
Contributor

I think the testing steps for this would be to try to register an account with email, adding spaces to the end.

Thanks, I'll do it and I'll create an issue for it first of all

@duxovni
Copy link
Contributor

duxovni commented Jul 7, 2022

Would a simpler approach be to just tweak that form field in the UI so it ignores any attempts to input whitespace? As in, you press space and the field value doesn't change, you paste in a string containing spaces and the spaces get automatically removed. That should solve this issue without requiring code changes elsewhere, right? It also feels more syntactically correct in some ways; rather than accepting strings that aren't valid email addresses, we just force the input to be a valid email.

@ankur12-1610
Copy link
Contributor

Would a simpler approach be to just tweak that form field in the UI so it ignores any attempts to input whitespace? As in, you press space and the field value doesn't change, you paste in a string containing spaces and the spaces get automatically removed. That should solve this issue without requiring code changes elsewhere, right? It also feels more syntactically correct in some ways; rather than accepting strings that aren't valid email addresses, we just force the input to be a valid email.

Yeah, actually I didn't get the chance to look at it, I'll look at it now.

This sounds a good idea to me I'll try to add some jquery or something like that so that even if the user enters the input it will convert it to null.

@turt2live
Copy link
Member

We don't have a jquery dependency and wouldn't really want to add one to be honest - we can just keep trim() on the field handling.

@ankur12-1610
Copy link
Contributor

We don't have a jquery dependency and wouldn't really want to add one to be honest - we can just keep trim() on the field handling.

Ok! I'll go with trim() then

@ankur12-1610
Copy link
Contributor

I've raised the PR which solves this issue and is the continuation of this PR.
PR: #9027

@turt2live
Copy link
Member

Closing in favour of #9027

@turt2live turt2live closed this Jul 8, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
T-Enhancement New features, changes in functionality, performance boosts, user-facing improvements Z-Community-PR Issue is solved by a community member's PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants