Page MenuHomePhabricator

HTML emails for Calendar event description changes should respect remarkup rules
ClosedPublic

Authored by lpriestley on Jul 5 2015, 5:26 PM.
Tags
None
Referenced Files
Unknown Object (File)
Thu, Apr 25, 12:06 AM
Unknown Object (File)
Wed, Apr 24, 3:49 AM
Unknown Object (File)
Thu, Apr 18, 2:57 PM
Unknown Object (File)
Thu, Apr 11, 8:23 AM
Unknown Object (File)
Sun, Apr 7, 9:25 PM
Unknown Object (File)
Mon, Apr 1, 11:37 AM
Unknown Object (File)
Sun, Mar 31, 7:58 PM
Unknown Object (File)
Fri, Mar 29, 1:16 PM
Subscribers

Details

Summary

Ref T7964, HTML emails for Calendar event description changes should respect remarkup rules

Test Plan

Create event and edit description, check that email has a correctly formatted remarkup description section.

Diff Detail

Repository
rP Phabricator
Branch
calendardescriptionemail
Lint
Lint Passed
Unit
Tests Passed
Build Status
Buildable 7146
Build 7340: [Placeholder Plan] Wait for 30 Seconds
Build 7339: arc lint + arc unit

Event Timeline

lpriestley retitled this revision from to HTML emails for Calendar event description changes should respect remarkup rules.
lpriestley updated this object.
lpriestley edited the test plan for this revision. (Show Details)
lpriestley added a reviewer: epriestley.
epriestley edited edge metadata.

Sorry, I think I led you astray a bit here.

I think the call to addRemarkupSection() in PhabricatorApplicationTransactionEditor also needs to be converted.

src/applications/metamta/view/PhabricatorMetaMTAMailBody.php
46–49

I think we can actually toss this..

56–64

...and do addPlaintextSection() here. As written, this isn't correct -- it adds HTML to the "plaintext" sections.

78

..and addHTMLSection(...) here.

89

...and addHTMLSection() here too.

147

Then this can be left as-is..

247

And this can be removed.

This revision now requires changes to proceed.Jul 6 2015, 6:08 PM
src/applications/metamta/view/PhabricatorMetaMTAMailBody.php
56–64

But it previously added "styled_text"? Seems counterintuitive to add plaintext by default in a remarkup section?

lpriestley edited edge metadata.

Updating PhabricatorApplicationTransactionEditor as well

epriestley edited edge metadata.

This looks correct to me, except that I think the bold for headers got stripped out and never gets added now?

src/applications/metamta/view/PhabricatorMetaMTAMailBody.php
157

It looks like we lost the bold here?

This revision now requires changes to proceed.Nov 4 2015, 6:42 PM
src/applications/metamta/view/PhabricatorMetaMTAMailBody.php
157

Trying to recall why I removed the phutil_tag. I think this was intentional. What's the right way to make this bold without a phutil_tag?

There's no way to make it bold without a phutil_tag().

My guess is that you might have removed it because the header is sometimes null, so it would have rendered an empty tag. Maybe something like this?

if ($header !== null) {
  $header = phutil_tag(...);
}

...but I'm not sure.

lpriestley edited edge metadata.

Adding back bold headers to mail

epriestley edited edge metadata.
This revision is now accepted and ready to land.Nov 7 2015, 3:23 PM
This revision was automatically updated to reflect the committed changes.