Page MenuHomePhabricator

Transactions - make edit transactions that are grouped work nicely
ClosedPublic

Authored by btrahan on Apr 4 2014, 5:50 PM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, Apr 5, 11:35 PM
Unknown Object (File)
Wed, Apr 3, 11:32 PM
Unknown Object (File)
Mon, Apr 1, 11:33 PM
Unknown Object (File)
Fri, Mar 29, 1:01 PM
Unknown Object (File)
Mar 22 2024, 11:53 PM
Unknown Object (File)
Mar 22 2024, 11:53 PM
Unknown Object (File)
Mar 22 2024, 11:53 PM
Unknown Object (File)
Mar 22 2024, 11:53 PM

Details

Summary

...the key is to move a layer lower and beam down the updated comment. There is a wee bit of Javascript gymnastics going on here. Fixes T4608.

Test Plan

made a comment + resolve. clicked edit and made changes. noted transaction updated correctly and "history" link worked. edited again to a deletion and noted the "this is deleted" looked right and history link still worked

Diff Detail

Repository
rP Phabricator
Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

btrahan retitled this revision from to Transactions - make edit transactions that are grouped work nicely.
btrahan updated this object.
btrahan edited the test plan for this revision. (Show Details)
btrahan added a reviewer: epriestley.

move funky chars to a constant that send to JS land via config
include javelin-util and rebuild celerity

Does this work properly with inlines in Differential and Pholio? Those both override renderTransactionContent() but it looks like they might not be accounted for?

This looks good to me otherwise, but I think that needs to be tweaked a bit.

I'm not sure; will check that out.

btrahan edited edge metadata.

make these changes diffusion / pholio aware

  • revert "Content" => "Comment" since it makes more sense as "Content" in broader context
  • update pholio css ever so slightly to style new wrapper span correctly.
  • update JS ever so slightly to expect a "set" of comments, and just use the first one.
epriestley edited edge metadata.
This revision is now accepted and ready to land.Apr 4 2014, 7:18 PM
btrahan updated this revision to Diff 20648.

Closed by commit rP496a7d8967e2 (authored by @btrahan).