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
F14481832: D13554.id34870.diff
Mon, Dec 30, 5:38 AM
Unknown Object (File)
Sat, Dec 28, 4:28 PM
Unknown Object (File)
Wed, Dec 25, 5:49 PM
Unknown Object (File)
Tue, Dec 17, 5:54 AM
Unknown Object (File)
Thu, Dec 12, 7:38 PM
Unknown Object (File)
Thu, Dec 12, 3:08 AM
Unknown Object (File)
Tue, Dec 10, 6:39 PM
Unknown Object (File)
Mon, Dec 2, 9:38 AM
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 7243
Build 7528: [Placeholder Plan] Wait for 30 Seconds
Build 7527: 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..

51–52

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

65

..and addHTMLSection(...) here.

67–68

...and addHTMLSection() here too.

119

Then this can be left as-is..

213

And this can be removed.

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

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
124

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
124

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.