Summary: we should build all the image stuff on every post and use that posted image data if there's an error. this diff makes that so. Fixes T4380.
Test Plan: made a mock with no title, tried to save it, and was delighted to see my images still there. edited a mock - removing the title and adding images - verified edits showed up after erroneous submission. added a title and submitted and changes were saved.
Reviewers: epriestley
Reviewed By: epriestley
CC: Korvin, epriestley, aran
Maniphest Tasks: T4380
Differential Revision: https://secure.phabricator.com/D8197
Summary: Similar to D8110, but for Pholio. Also an IRC user request.
Test Plan: Set setting to something unusual, created a new mock.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D8111
Summary: This removes the bulk of the "Form Errors" text, some variations likely exists. These are a bit redundant and space consuming. I'd also like to back ErrorView more into PHUIObjectBox.
Test Plan: Test out the forms, see errors without the text.
Reviewers: epriestley, btrahan
CC: Korvin, epriestley, aran, hach-que
Differential Revision: https://secure.phabricator.com/D7924
Summary: We currently have a lot of calls to `addCrumb(id(new PhabricatorCrumbView())->...)` which can be expressed much more simply with a convenience method. Nearly all crumbs are only textual.
Test Plan:
- This was mostly automated, then I cleaned up a few unusual sites manually.
- Bunch of grep / randomly clicking around.
Reviewers: btrahan, chad
Reviewed By: btrahan
CC: hach-que, aran
Differential Revision: https://secure.phabricator.com/D7787
Summary: Ref T1279. Prerequisite for adding icons or other type information to tokenizers, since we don't currently have enough information to prefill them when rendering things from the server side. By passing handles in, the tokenizer can extract type information.
Test Plan:
- Searched by user in Audit.
- Sent Conpherence from profile page.
- Tried to send an empty conpherence.
- Searched Countdown by user.
- Edited CCs in Differential.
- Edited reviewers in Differential.
- Edited a commit's projects.
- Searched lint by owner.
- Searched feed by owner/project.
- Searched files by owner.
- Searched Herald by owner.
- Searched Legalpad by owner.
- Searched Macro by owner.
- Filtered Maniphest reports by project.
- Edited CCs in Maniphest.
- Searched Owners by owner.
- Edited an Owners package.
- Searched Paste by owner.
- Searched activity logs by owner.
- Searched for mocks by owner.
- Edited a mock's CCs.
- Searched Ponder by owner.
- Searched projects by owner.
- Edited a Releeph project's pushers.
- Searched Releeph by requestor.
- Edited "Uses Symbols" for an Arcanist project.
- Edited all tokenizers in main search.
- Searched Slowvote by user.
Reviewers: chad, btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T1279
Differential Revision: https://secure.phabricator.com/D7248
Summary: I'd like to reuse this for other content areas, renaming for now. This might be weird to keep setForm, but I can fix that later if we need.
Test Plan: reload a few forms in maniphest, projects, differential
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: Korvin, aran
Differential Revision: https://secure.phabricator.com/D7120
Summary: Ref T603. Killing this class is cool because the classes that replace it are policy-aware. Tried to keep my wits about me as I did this and fixed a few random things along the way. (Ones I remember right now are pulling a query outside of a foreach loop in Releeph and fixing the text in UIExample to note that the ace of hearts if "a powerful" card and not the "most powerful" card (Q of spades gets that honor IMO))
Test Plan: tested the first few changes (execute, executeOne X handle, object) then got real mechanical / careful with the other changes.
Reviewers: epriestley
Reviewed By: epriestley
CC: Korvin, aran, FacebookPOC
Maniphest Tasks: T603
Differential Revision: https://secure.phabricator.com/D6941
Summary:
This attempts some consistency in form layouts. Notably, they all now contain headers and are 16px off the sides and tops of pages. Also updated dialogs to the same look and feel. I think I got 98% of forms with this pass, but it's likely I missed some buried somewhere.
TODO: will take another pass as consolidating these colors and new gradients in another diff.
Test Plan: Played in my sandbox all week. Please play with it too and let me know how they feel.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: Korvin, aran
Differential Revision: https://secure.phabricator.com/D6806
Summary:
companion diff to D6729. This is the back-end stuff, plus calls the JS in D6729 for when images are removed, un-removed, uploaded, or replaced.
Fixes T3640.
Test Plan: messed around with images. hit save - new order! temporarily showed these stories and got text about re-ordering stuff.
Reviewers: epriestley, chad
Reviewed By: epriestley
CC: Korvin, aran
Maniphest Tasks: T3640
Differential Revision: https://secure.phabricator.com/D6731
Summary:
Ref T3640. JS part only, should give you a list in `imageOrder` on the server that you can read with `$request->getStrList('imageOrder')`.
NOTE: You can't drag images into the first position; this is an existing thing that I just need to fix with DraggableList.
@chad might have some design feedback.
Test Plan: Dragged images around, things seemed to work?
Reviewers: btrahan, chad
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T3640
Differential Revision: https://secure.phabricator.com/D6729
Summary:
Now you can actually replace an image! Ref T3572. This ended up needing a wee bit of infrastructure to work...
- add replace image transaction to pholio
- add replacesImagePHID to PholioImage
- tweaks to editor to properly update images with respect to replacement
- add edges to track replacement
- expose getNodes on graph query infrastructure to query the entire graph of who replaced who
- move pholio image to new phid infrastructure
Still TODO - the history view should get chopped out a bit from the current view - no more inline comments / generally less functionality plus maybe a tweak or two to make this more sensical.
Test Plan: replaced images and played with history controller a little. works okay.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T3572
Differential Revision: https://secure.phabricator.com/D6560
Summary: Ref T2637. Ref T2625. Convert Pholio to use ApplicationSearch.
Test Plan:
Viewed mocks, changed filters.
Verified edit page works and no longer has a weird side nav on it.
Reviewers: btrahan, chad
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2625, T2637
Differential Revision: https://secure.phabricator.com/D6530
Summary: Ref T2637. Allows you to "undo" if you delete an image from a mock by accident.
Test Plan:
Deleted; undo'd.
{F50878}
Reviewers: btrahan, chad
Reviewed By: chad
CC: aran
Maniphest Tasks: T2637
Differential Revision: https://secure.phabricator.com/D6498
Summary: Fixes T3553. Did it by adding some code that refreshes the File object on keyup events within a given file entry. also fixes an html derp I found trying to fix this.
Test Plan: added cool things like 'bbb' to every field and noted they were maintained when I added more files
Reviewers: btrahan
Reviewed By: btrahan
CC: aran, Korvin, chad
Maniphest Tasks: T3553
Differential Revision: https://secure.phabricator.com/D6488
Summary: Fixes T3573 and T3541. (Note T3573 is only fixed moving forward as I changed how transactions are converted in the editor.)
Test Plan: created mocks with combinations of set and empty image title and descriptions. verified when i updated title / description transactions showed up. called up the email via command line tools and saw nice 'created' text.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T3541, T3573
Differential Revision: https://secure.phabricator.com/D6495
Summary:
Nice title. We add three new transactions - IMAGE_FILE, IMAGE_NAME, and IMAGE_DESCRIPTION. The first is a bit like subscribers as it is a list of file phids. The latter have values of the form ($file_phid => $data), where $data is $name or $description respectively. This is because we need to collate transactions based on $file_phid...
Overall, this uses the _underyling files_ and not the "PholioImage" to determine if things are unique or not. That said, simply mark PholioImages as obsolete so inline comments about no-longer applicable PholioImages don't break.
Does a reasonable job implementing the mock. Note you can't "update" an image at this time, though you can delete and add at will.
Test Plan: played with pholio a ton.
Reviewers: epriestley
Reviewed By: epriestley
CC: chad, aran, Korvin
Maniphest Tasks: T3489
Differential Revision: https://secure.phabricator.com/D6441
Summary: I introduced this helper at some point, clean up all the code duplication around content sources.
Test Plan: Grepped; hit edit interfaces for most/all of these.
Reviewers: btrahan, chad, edward
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D6030
Summary: Not for full review. This makes crumbs appear consistently in mobile. It helps give a quick link to the apps home, the page title currently on, and action icons for the object. It will take additional clean-up to make this consistent across apps. Passing for early review from a UEX perspective. I actually really like it and think onces it's everywhere, helps mobile feel complete.
Test Plan: Testing in iOS and Simulator.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2796
Differential Revision: https://secure.phabricator.com/D5446
Summary: Adds the hover color to the upload images area on Create Mock.
Test Plan: Hover image, get background change.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin, AnhNhan
Differential Revision: https://secure.phabricator.com/D5291
Summary: For now, remove the images field when editing a mock, can add it back if we decide to allow it.
Test Plan: Check edit mock, field gone, check new mock, field is alive and well.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2552
Differential Revision: https://secure.phabricator.com/D5289
Summary: good title
Test Plan: made a mock with no title and the files i had attached showed up again correctly. named the mock and it worked!
Reviewers: epriestley, chad
Reviewed By: chad
CC: aran, Korvin
Maniphest Tasks: T2642
Differential Revision: https://secure.phabricator.com/D5261
Summary: Not sure if we're going to do reviewers or whatever, but let mock creators share mocks with lists or individuals.
Test Plan: Created a mock with "duck" and "dog" cc'd, verified they showed up on the email.
Reviewers: chad
Reviewed By: chad
CC: aran
Maniphest Tasks: T2568
Differential Revision: https://secure.phabricator.com/D5056
Summary:
Implements previews for Macros and Pholio.
(Design is nonfinal -- kind of split the difference between `diff_full_view.png`, laziness, and space concerns. Next couple diffs will add more stuff here.)
Test Plan: {F28055}
Reviewers: btrahan, chad
Reviewed By: btrahan
CC: aran, vrana
Maniphest Tasks: T2104
Differential Revision: https://secure.phabricator.com/D4246
Summary:
When a user submits an action with no effect (like an empty comment, an "abandon" on an already-accepted revision, or a "close, resolved" on a closed task) we want to alert them that their action isn't effective. These warnings fall into two general buckets:
- User is submitting two or more actions, and some aren't effective but some are. Prompt them to apply the effective actions only.
- A special case of this is where the only effective action is a comment. We provide tailored text ("Post Comment") in this case.
- User is submitting one action, which isn't effective. Tell them they're out of luck.
- A special case of this is an empty comment. We provide tailored text in this case.
By default, the transaction editor throws when transactions have no effect. The caller can then deal with this, or use `PhabricatorApplicationTransactionNoEffectResponse` to provide a standard dialog that gives the user information as above. For cases where we expect transactions to have no effect (notably, "edit" forms) we just continue on no-effect unconditionally.
Also fix an issue where new, combined or filtered transactions would not be represented properly in the Ajax response (i.e., return final transactions from `applyTransactions()`), and translate some strings.
Test Plan:
- Submitted empty and nonempy comments in Macro and Pholio.
- Submitted comments with new and existing "@mentions".
- Submitted edits in both applications.
Reviewers: btrahan, vrana
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T912, T2104
Differential Revision: https://secure.phabricator.com/D4160
Summary:
Split Pholio's transaction implementation into generic and application-specific parts. Moves us toward generic transactions, with support for:
- Editing and deleting comments.
- Setting visibility of individual comments (I'm not a fan of this feature but we'll see).
I want to move everything to a more generic piece of infrastructure but there's very little they can share right now so adding transactions to, e.g., Paste or Macros (T2157) means massive amounts of similar code.
Tons of work left to do here, but I think it basically works. Here's a screenshot:
{F26820}
Test Plan: Made transactions in Pholio.
Reviewers: btrahan, vrana, chad
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2104
Differential Revision: https://secure.phabricator.com/D4136
Summary:
Some users like monospaced textareas and others don't.
This introduces an option to set this as a user preference.
Test Plan: Enabled and saw monospaced textareas, disabled and saw non-monospaced textareas.
Reviewers: epriestley
CC: aran, Korvin
Maniphest Tasks: T2114
Differential Revision: https://secure.phabricator.com/D4037
Summary:
- Use transactions to apply edits.
- Use Editor to apply transactions.
- Some special casing for tricky stuff I don't want to deal with yet (mock images).
Test Plan: {F22368}
Reviewers: btrahan, chad
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2097
Differential Revision: https://secure.phabricator.com/D3837
Summary:
Nothing app-specific yet, just stitching groundwork together from readymade components.
{F22347}
Test Plan: Created some mocks via web UI.
Reviewers: btrahan, chad
Reviewed By: chad
CC: aran
Maniphest Tasks: T2097
Differential Revision: https://secure.phabricator.com/D3830
