Summary:
Fixes T8323. See that task for a description.
We were using `nonempty()`, but that rule doesn't cover synthetic deletions (file present in an earlier diff, but no longer present in the later diff).
Test Plan: Followed the steps in T8323, got a clean comment.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T8323
Differential Revision: https://secure.phabricator.com/D17929
Summary: Ref T12616. This makes line range selection use the new code, and removes the remainder of the old "hover a line number" / "select a line range" code.
Test Plan: Hovered line numbers; selected line ranges.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T12616
Differential Revision: https://secure.phabricator.com/D17927
Summary:
Fixes T7682. The left-hand-side "<th />" row did not generate with the correct ID.
(I couldn't reproduce the exact issue described in T7682, but hovering comments on either side now works properly for me.)
Test Plan: {F4962479}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T7682
Differential Revision: https://secure.phabricator.com/D17926
Summary:
Ref T11401. Fixes T5232. Ref T12616.
Partly, this moves more code over to the new stuff.
This also allows "r" to work if you have code selected (not just comments). If you "reply" to code, you start a new comment.
You can "R" a comment to quote it. This just starts a new comment normally if you "R" a block of code. This is sort of a power-user version of "quote" since it seems like it probably doesn't really make sense to put it in the UI ever (maybe).
With the new click-to-select, you can click + "R" to reply-with-quote.
Test Plan: Used "r" and "R" to reply to comments and code.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T12616, T11401, T5232
Differential Revision: https://secure.phabricator.com/D17920
Summary: Moves this transaction over to modular transactions.
Test Plan: Move a document, re-title a document, try to move over an existing document.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D17918
Summary: It's an icon. For story points.
Test Plan: Set some points, see icon.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D17915
Summary: Run through all the pages in projects and make sure they all feel similar. Adds back curtain on board manage page, even though it is sad for only having a single action.
Test Plan: Test all pages on a project for consistency in UI.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D17909
Summary:
Ref T12616. Fixes T12715. I suspect these are very rarely used. (I think you tried to get rid of them before but I pushed back since we couldn't really offer great alternatives at the time?)
Now that the code is in a better place:
- Click an inline's header (just the colored part) to select it with the keyboard selection cursor.
- Click again to deselect it.
- You can use "n" and "p" to jump to comments, so "click + n" is the same as the old "V" action.
- This also makes it easier to swap between keyboard and mouse workflows, since you can jump into things with the keyboard at any inline.
Also, make "Reply" render more consistently.
Test Plan:
- Did all that stuff, things seemed to work OK.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T12715, T12616
Differential Revision: https://secure.phabricator.com/D17908
Summary: Cleans up the UI, moves details over to curtain, adds some fallback no data strings.
Test Plan: Review with and without subprojects, milestones.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D17907
Summary: We seem to already support this, just takes it fully there. We don't need to see things like "Flag", etc, on certain subpages of projects/people/etc.
Test Plan: Review Members, Subproject pages, no longer see "Flag for Later" which only is for the Project itself. Check manage, still there.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D17897
Summary:
Fixes T8130. Allows selected comments to be shown/hidden (with "q") or marked done/not-done (with "w").
(These key selections are because "qwer" are right next to each other on QWERTY keyboards, and now mean "hide, done, edit, reply".)
Also, allow "N" and "P" to do next/previous inline, including hidden inlines. This makes "q" to hide/show a little more powerful and a little easier to undo.
Test Plan: Used "q", "w", "N" and "P" to navigate and interact with comments.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T8130
Differential Revision: https://secure.phabricator.com/D17906
Summary: Restricts the view of the membership privileges to just the Members page itself, and not other pages like Home/Details.
Test Plan: Test Home, Test Members, see correct layouts.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D17896
Summary: This UI can use the setDrag call to reduce clutter on the reodering dialog.
Test Plan:
Reorder some columns, save.
{F4959906}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D17898
Summary: Ref T12616. This makes "edit" and "reply" work again.
Test Plan:
Used "e" and "r" to edit and reply.
Also used them in bogus ways and got useful UI feedback.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T12616
Differential Revision: https://secure.phabricator.com/D17895
Summary: Ref T12616. This moves most keyboard shortcuts into DiffChangesetList. It breaks some shortcuts that I plan to restore later, noted in T12616 (toggle file, edit inline, reply to inline), since I think ripping them out now and rebuilding them in a little bit will make things much simpler.
Test Plan:
- Used j, k, n, p, J, K shortcuts to navigate a revision.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T12616
Differential Revision: https://secure.phabricator.com/D17859
Summary:
Ref T12616. Fixes T12153. Currently, when you hide inlines, they hide completely and turn into a little bubble on the previous line.
Instead, collapse them to a single line one-by-one. Narrowly, this fixes T12153.
In the future, I plan to make these changes so this feature makes more sense:
- Introduce global "hide everything" states (T8909) so you can completely hide stuff if you want, and this represents more of a halfway state between "nuke it" and "view it".
- Make the actual rendering better, so it says "epriestley: blah blah..." instead of just "..." -- and looks less dumb.
The real goal here is to introduce `DiffInline` and continue moving stuff from the tangled jungle of a million top-level behaviors to sensible smooth statefulness.
Test Plan:
- Hid and revealed inlines in unified and two-up modes.
- These look pretty junk for now:
{F4948659}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T12616, T12153
Differential Revision: https://secure.phabricator.com/D17861
Summary: Ref T12616. This cements the relationship between ChangesetList (parent container) and Changeset (child) and passes translations down so Changeset can use them to translate the text "Loading..."
Test Plan: Viewed loading changes.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T12616
Differential Revision: https://secure.phabricator.com/D17846
Summary: Ref T12616. This ends up being a little messy ("one giant function") and maybe I'll clean it up a bit later, but continue consolidating the wild jungle of behaviors into a smaller set of responsible objects.
Test Plan: Clicked all the menu options, saw them work properly. Grepped for removed methods.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T12616
Differential Revision: https://secure.phabricator.com/D17845
Summary: Also cleans up now-dead code relating to old transactions
Test Plan: Created lots of mocks, replaced their images, added/removed images, changed the sequence, verified expected DB xaction rows, Mock updates, and correct rendering of timeline.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: epriestley
Differential Revision: https://secure.phabricator.com/D17892
Summary: Various little fixes, mostly moves information from the "Details" section either into the curtain or into the specific watchers or members list based on user viewership. I think this page is both cleaner and more informative.
Test Plan:
Lock, Unlock, Watch, Join, various projects with multiple users.
{F4959101}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D17891
Summary: Removal of `PholioMockEditor::applyCustomInternalTransaction()` in D17868 broke creation of new mocks in Pholio. Puts the empty method back until we finish migrating Pholio to modular transactions.
Test Plan: Created some mocks, observed lack of unhandled exception.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: epriestley
Differential Revision: https://secure.phabricator.com/D17889
Summary: I think this is the correct fix, sets a consistent value for transactions, old and new, for Maniphest point values.
Test Plan:
Edit title, see no point feed story, set points, see point story, set points to same value, see no story, remove points, see remove point story.
{F4958233}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D17885
Summary: Fixes T12713. We don't need to show watching and member info on other views other than ApplicationSearch (for now) so add a few methods to restrict the calls.
Test Plan: Visit project search, profile, project home, project home with subprojects
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T12713
Differential Revision: https://secure.phabricator.com/D17883
Summary: Slightly nicer, more consistent UI. Also removed "Column History" from dropdowns as this is available on the general board manage page.
Test Plan: Review Board and Column management pages.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D17881
Summary:
Fixes T12710. See that task for discussion. This is pretty ugly/redundant but not broken.
(Feel free to reject this and pursue something else.)
Test Plan:
- For a project with active subprojects/milestones, viewed the project profile and subprojects tabs.
- After patch: they're ugly, but no longer fatal.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T12710
Differential Revision: https://secure.phabricator.com/D17882
Summary: Moves "reorder columns" and "change background" up a level, redesigns "manage" page to be a little cleaner.
Test Plan: Change colors, reorder columns, manage page, disable board, re-enable board.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D17879
Summary: Fixes T12707 Adds additional information to search results for if user is a member or a watcher of a project. Also removed the icon colors, which I'll find a better way to denote in future.
Test Plan: Join a project, watch a project, view results list in /projects/
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T12707
Differential Revision: https://secure.phabricator.com/D17880
Summary:
Ref T12685.
- Better icon/color/label consistency.
- Make "0" a valid response.
- Fix a bug where creating a poll with Quicksand enabled led to bad times (form submitted back to the search screen).
Test Plan: {F4956412}
Reviewers: chad, amckinley
Reviewed By: chad
Maniphest Tasks: T12685
Differential Revision: https://secure.phabricator.com/D17878
Summary:
Ref T12685. When you edit an application's policies but don't make any changes, you currently get stuck on the same page. This isn't how other edit screens work, and I think it's a holdover from eras long ago.
Make it consistent with other applications.
Also, fix a missing `pht()`.
Test Plan:
- Edited an application configuation.
- Clicked "Save" without making changes.
- After patch, was redirected back to detail page like in other applications.
Reviewers: chad, amckinley
Reviewed By: chad
Maniphest Tasks: T12685
Differential Revision: https://secure.phabricator.com/D17877
Summary:
Ref T12685. I provided this incorrect (`return new` rather than `throw`) implementation earlier; it can now be replaced with a proper implementation.
This caused application policy edits to spew this into the daemon log:
```
[2017-05-14 15:35:27] EXCEPTION: (Error) Call to undefined method PhutilMethodNotImplementedException::setActor() at [<phabricator>/src/applications/transactions/worker/PhabricatorApplicationTransactionPublishWorker.php:69]
```
Test Plan:
- Used `bin/worker execute --id <id>` to execute a previously-failing task.
- Saw a feed story publish.
Reviewers: chad, amckinley
Reviewed By: chad
Maniphest Tasks: T12685
Differential Revision: https://secure.phabricator.com/D17876
Summary: This brings up "Edit Column" as an action item under the main column dropdown as well as a "Column History" for completeness. Unsure column history is actually useful, but leaving it in anyways. It might be nice to have some sort of dialog version of a history page.
Test Plan: Make a workboard, add a column, edit column name, stay on workboard.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: epriestley
Differential Revision: https://secure.phabricator.com/D17874
Summary:
Fixes T12679. Reproduction steps appear to be:
- As a logged-out user, view revision list or commit list.
- Enable bucketing by action required.
- Before patch: `foreach (null as ...)` causes error spew.
- After patch: `foreach (array() as ...)` works great.
Test Plan:
- Reproduced issue by following steps above in Differential (revisions) and Diffusion (audits/commits).
- After patches, no more errors in the log.
Reviewers: chad, amckinley
Reviewed By: chad
Maniphest Tasks: T12679
Differential Revision: https://secure.phabricator.com/D17872
Summary: Ref T12707. Adds a simple filter for the viewer if logged in.
Test Plan: Watch a project, click on watching list, see project I'm watching.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: epriestley
Maniphest Tasks: T12707
Differential Revision: https://secure.phabricator.com/D17873
Summary: Also removes more now-dead code from `PholioTransaction`.
Test Plan: opened and closed a bunch of mocks, edited a bunch of image descriptions
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D17868
Summary: Caught this in the logs, calling an old transaction, update it.
Test Plan: Answer a question, tail log, see no error.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D17870
Summary: Trailing logs for errors here, picked up some unset constants in Phame.
Test Plan: New Blog, New Post, tail daemon log for errors. See feed stories.
Reviewers: epriestley, amckinley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D17871
Summary: The ports over a similar "profile image" menu item to Projects. It gives us some room to use the project icon in the sidenav along with a larger photo. It also will open up some room in the sub-page headers for us to focus on that page, and not the identity of the project at hand. Expect a few more project related touch up diffs.
Test Plan:
Review new projects menu on a few projects, update the image, see new image. Great for team photos.
{F4951264}
Reviewers: epriestley, amckinley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D17869
Summary: Another Franken-transaction. Adds transactions for ImageName and MockName. Adds transaction-level validation for presence of mock name; removed same from edit controller. Removes `PholioTransaction::getRemarkupBodyForFeed()`, which appears to be dead code since that method isn't defined on any other types.
Test Plan: made a bunch of changes to pholio mocks and images and observed expected results
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: Korvin, chad, epriestley
Differential Revision: https://secure.phabricator.com/D17865
Summary: Updates Legalpad to use EditEngine, paving the way for //transaction comments//. Spooky.
Test Plan:
- New Document
- Require signing, Corp - see fail
- Require signing, Noone - see fail
- Require signing, Ind - get asked to sign
- Edit Document
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D17862
Summary: Begins the process of migrating Pholio to Modular Transactions by starting with the mock's description and changing the base class of PholioTransaction. Expect several more of these diffs to quickly follow. Also changes the icon for description changes to `fa-pencil`; previously it was check or ban, depending on the open/closed state. Looks like an accidental switch fallthrough.
Test Plan: made new mocks, edited their descriptions, observed same UI as previously
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D17864
Summary: This function `renderHandleLink` doesn't exist. Update call.
Test Plan: I'm still lost on how to refund a transaction? Existing?
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D17858
Summary: Ref T12685. Moves Fund to EditEngine commenting.
Test Plan: View an old Fund, leave a comment, add a subscriber.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T12685
Differential Revision: https://secure.phabricator.com/D17857
Summary: Ref T12685, updates fund for edit engine.
Test Plan: Create a Fund, Edit a Fund, wipe out Merchants, check errors for name and missing merchants, back Fund.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T12685
Differential Revision: https://secure.phabricator.com/D17855
