Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

1755: clickable email addresses #1856

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

bahaaTuffaha
Copy link

Short description

The e-mail addresses should be clickable so that they do not have to be copied and pasted.

Proposed changes

  • Modifying JsonFieldElemental to check string if its a valid email address to show is as a mail:to link.
  • verificationHelper.tsx having the regex for email verification function at bp-modules/applications/utils

Side effects

  • JsonFieldElemental.tsx modified.

Testing

  • Login as admin for Ehrenamtskarte Bayern region 9
  • Make sure there is a card request needs review.
  • Check email details

Fixes: #1755

Copy link
Contributor

@f1sh1918 f1sh1918 left a comment

Choose a reason for hiding this comment

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

Congratulation to your first PR! Very nice work! 👍
Works like a charm. Tested on chrome

Just found some small improvements, i commented

One thing that was probably not mentioned in the issue was that in the organization section (screenshot) the mail address should also be clickable since the service workers may contact the organization for further questions. You can either add it in this pr or create a new issue for it.
image

@@ -0,0 +1,4 @@
export const isEmailValid = (email: string): boolean => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
export const isEmailValid = (email: string): boolean => {
export const isEmailValid = (email: string): boolean => /^[^\s@]+@[^\s@]+\.[^\s@]+$/.test(email)

🔧 Since you are not reusing the pattern, this one liner should be enough

export const isEmailValid = (email: string): boolean => {
const pattern = /^[^\s@]+@[^\s@]+\.[^\s@]+$/
return pattern.test(email)
}
Copy link
Contributor

Choose a reason for hiding this comment

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

❌ Please rename the file to .ts since you are not using any tsx syntax like templating

{t(getTranslationKey())}: {jsonField.value}
{t(getTranslationKey())}:
{isEmailValid(jsonField.value) ? (
<a href={`mailto:${jsonField.value}`}>{jsonField.value}</a>
Copy link
Contributor

@f1sh1918 f1sh1918 Jan 9, 2025

Choose a reason for hiding this comment

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

Suggested change
<a href={`mailto:${jsonField.value}`}>{jsonField.value}</a>
<a href={`mailto:${jsonField.value}`}> {jsonField.value}</a>

🙃 i think there is an empty space missing after the label and before the mail address

Copy link
Author

Choose a reason for hiding this comment

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

I noticed it but I kept the same format as possible...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Make email addresses in the application object clickable
2 participants