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

Add subject and body to mailto links #10920

Merged
merged 1 commit into from Sep 26, 2023
Merged

Add subject and body to mailto links #10920

merged 1 commit into from Sep 26, 2023

Conversation

TopDevPros
Copy link
Contributor

@TopDevPros TopDevPros commented Sep 20, 2023

Fixes #5950. Updated code from PR #6451 which was abandoned 3 years ago. Added more comprehensive tests.

[Y] Do the tests still pass?
[Y] Does the code comply with the style guide?
    [Y] Run make lint from the Wagtail root.
[Y] For Python changes: Have you added tests to cover the new/fixed behaviour?
[Y] For front-end changes: Did you test on all of Wagtail’s supported environments?
    Chromium and Firefox on linux
[N] For new features: Has the documentation been updated accordingly?

    The only possibly related docs are in wagtail/docs/extending/rich_text_internals.md. 
    It appears that we would unnecessarily complicate the current docs with details about
    the new optional fields.

@squash-labs
Copy link

squash-labs bot commented Sep 20, 2023

Manage this branch in Squash

Test this branch here: https://topdevpros5950-au4a8.squash.io

@TopDevPros TopDevPros changed the title 5950 Add subject and body to mailto links Sep 20, 2023
@lb- lb- added status:Needs Review component:Choosers Modal choosers for Page, Snippet and other models labels Sep 25, 2023
Copy link
Member

@lb- lb- left a comment

Choose a reason for hiding this comment

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

This works well, thank you. Thanks also for adding robust tests.

I have one line of feedback, but can add this easily in the PR when merging.

Screenshot 2023-09-26 at 3 35 32 pm

When you save an email address, then edit, then switch to a non-email field, you get the raw link in the other sections. I think this is fine but just calling this out. I see that we do this for the reverse (kind of), when a non-email link has been added.

No action needed on this, just leaving notes for future self if we want to change this behaviour one day.

@@ -44,6 +44,8 @@ class AnchorLinkChooserForm(forms.Form):
class EmailLinkChooserForm(forms.Form):
email_address = forms.EmailField(required=True)
link_text = forms.CharField(required=False)
subject = forms.CharField(required=False)
body = forms.CharField(required=False)
Copy link
Member

Choose a reason for hiding this comment

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

Can we please make this work as a multi-line field, something like...

Suggested change
body = forms.CharField(required=False)
body = forms.CharField(required=False, widget=forms.Textarea(attrs={"rows": 3}))

@lb-
Copy link
Member

lb- commented Sep 26, 2023

Rebased/formatted files, added the textarea widget and pushed. Will review the CI and merge in if all good.

@lb- lb- merged commit 3ffe4cb into wagtail:main Sep 26, 2023
19 of 20 checks passed
@TopDevPros TopDevPros deleted the 5950 branch September 26, 2023 10:46
@TopDevPros
Copy link
Contributor Author

@lb, thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component:Choosers Modal choosers for Page, Snippet and other models
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unable to add parameters to mailto links
2 participants