Seeing if this is the correct path, then will apply in Pholio, Ponder.
Details
- Reviewers
epriestley - Maniphest Tasks
- T9825: Remarkup is rendered literally in Diviner
- Commits
- Restricted Diffusion Commit
rP8d62ade70a06: Render Remarkup poorly in Phame Feed stories
epriestley
Diff Detail
- Repository
- rP Phabricator
- Branch
- master
- Lint
Lint Passed - Unit
Tests Passed - Build Status
Buildable 9242 Build 10951: Run Core Tests Build 10950: arc lint + arc unit
Event Timeline
Try something like this instead, maybe?
In PhabricatorApplicationTransaction->getBodyForFeed(), add a clause near the top like this:
$remarkup = $this->getRemarkupBodyForFeed(); if ($remarkup !== null) { summarize; return new PHUIRemarkupView(...); }
Then add a new method:
protected function getMarkupBodyForFeed() { return null; }
Then rename PhameTransaction->getBodyForFeed() to getRemarkupBodyForFeed() and just have it return $text if it finds some text.
So the overall behavior is:
- getBodyForFeed(): Look for some remarkup by calling getRemarkupBodyForFeed(). If it finds some, summarize and render it and use that. Otherwise, do other stuff.
- getRemarkupBodyForFeed(): Return the relevant raw remarkup for this story, if some exists.
Then we can clean this up more later in T9789 etc.
Why do we need getBodyForFeed() and getRemarkupBodyForFeed()? This feels very verbose.
Because not all things which return a body can currently return raw remarkup.
Particularly, comments can return rendered remarkup much more efficiently than raw remarkup (the remarkup has already been batch-rendered earlier in the pipeline).
We could clean this up now, but we'd have to take a performance hit to do so, and no version of this will be particularly clean relative to T9789.
Even in the future, I think some stories are likely to contain bodies which are not remarkup. One example is that I'd like to show better human-readable diffs for "edited description of" stories; they would be HTML but not remarkup. I could also imagine "Edit profile picture" stories having non-remarkup bodies, etc.
At least since this is Phame/Pholio/Ponder, I don't expect a large number of stories.
We could also not do the getRemarkupBodyForFeed() thing and put some setSummarize(true) on PHUIRemarkupView and just have these things all do return new PHUIRemarkupView(...)->setSummarize(true), but that's doubling down on my ability to make that render efficiently later. I'm pretty sure I can, it's just a lot of eggs to put in the "anticipated performance improvement" basket.