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

SHS-5954: Social Media Footer block — allow mailto: links #1706

Merged
merged 7 commits into from
Jan 14, 2025

Conversation

codechefmarc
Copy link
Collaborator

@codechefmarc codechefmarc commented Jan 3, 2025

READY FOR REVIEW

Summary

Allows mailto: links in addition to standard URLs for social media block.

Need Review By (Date)

2025-01-08

Urgency

low

Steps to Test

  1. Add a social media block to the site by going to /admin/structure/block and placing the "Social Media Block" in the footer, for example.
  2. Under the "Links" section, for the URL, add in a mailto:[email protected] link
  3. Verify that the form saves correctly
  4. Edit the block and test out other variations - for example, malformed or incorrect mailto or URLs and verify there are correct error messages for the user.

Screenshot 2025-01-03 at 3 46 45 PM

Screenshot 2025-01-03 at 3 46 59 PM

PR Checklist


Copy link
Collaborator

@cienvaras cienvaras left a comment

Choose a reason for hiding this comment

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

@codechefmarc Works great, thanks!

@ahughes3 This one is ready for you, but tugboat is over quota so no test environment available yet. As soon as some older PR's get approved and merged we'll be able to create a test link.

@cienvaras cienvaras assigned ahughes3 and unassigned codechefmarc Jan 6, 2025
@cienvaras cienvaras requested a review from ahughes3 January 6, 2025 17:14
@ahughes3 ahughes3 temporarily deployed to Tugboat January 6, 2025 17:46 Destroyed
@ahughes3
Copy link
Collaborator

ahughes3 commented Jan 6, 2025

@cienvaras I rebuilt some of the older PRs in Tugboat which reduced their size by a few gigs and was able to get the previews for this PR built. Thank you!

Base automatically changed from 11.6.1-release to develop January 8, 2025 16:39
@ahughes3 ahughes3 assigned pookmish and unassigned ahughes3 Jan 13, 2025
@ahughes3 ahughes3 temporarily deployed to Tugboat January 13, 2025 19:49 Destroyed
Copy link
Collaborator

@ahughes3 ahughes3 left a comment

Choose a reason for hiding this comment

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

LGTM!

@ahughes3 ahughes3 temporarily deployed to Tugboat January 13, 2025 19:51 Destroyed
@ahughes3 ahughes3 temporarily deployed to Tugboat January 13, 2025 22:51 Destroyed
@codechefmarc codechefmarc changed the base branch from develop to 11.6.3-release January 13, 2025 22:51
@codechefmarc codechefmarc requested a review from pookmish January 13, 2025 23:23
@codechefmarc
Copy link
Collaborator Author

@pookmish - Ready for re-review. Thanks.

Copy link
Member

@pookmish pookmish left a comment

Choose a reason for hiding this comment

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

Try to avoid else statements if possible

Comment on lines +181 to +182
if (str_starts_with($value, 'mailto')) {
if (!preg_match($mailto_regex, $value)) {
Copy link
Member

Choose a reason for hiding this comment

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

Combine these two.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

One of the problems here is that if someone enters in mailto: with an invalid email address, that case will not be caught and instead will be validated as a URL. I can change so we don't get any custom messaging for invalid mailto links and combine if you wish.

Copy link
Member

Choose a reason for hiding this comment

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

ok, this works for me. 👍

@ahughes3 ahughes3 temporarily deployed to Tugboat January 14, 2025 01:22 Destroyed
@codechefmarc codechefmarc requested a review from pookmish January 14, 2025 01:41
@pookmish pookmish merged commit beb82ac into 11.6.3-release Jan 14, 2025
16 of 17 checks passed
@pookmish pookmish deleted the SHS-5954--social-allow-mailto branch January 14, 2025 16:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants