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, Nov 22, 10:37 PM
Unknown Object (File)
Thu, Nov 14, 5:49 PM
Unknown Object (File)
Sun, Nov 3, 12:29 PM
Unknown Object (File)
Fri, Nov 1, 1:45 PM
Unknown Object (File)
Oct 21 2024, 4:49 AM
Unknown Object (File)
Oct 19 2024, 3:51 AM
Unknown Object (File)
Oct 15 2024, 2:09 PM
Unknown Object (File)
Oct 3 2024, 12:54 AM

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
Branch
T4608
Lint
Lint Errors
Unit
Tests Passed

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).