Summary: The logs bits still need some work but add crumbs/lists to everything else. Also build a propery DrydockResourceQuery.
Test Plan: Looked at lease list/detail; resource list/detail.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2015
Differential Revision: https://secure.phabricator.com/D4221
Summary: arc incorrectly passes a "user" parameter to differential.createrevision (long ago, we respected it, I think). After D4191 this fatals. Provide a stub call until the next version bump.
Test Plan: inspection
Reviewers: vrana, btrahan
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D4220
Summary: Minor updates to Drydock things to make them work better. In particular, after this patch working copies are correctly allocated or reused.
Test Plan: Ran "reparse.php --harbormaster <derp derp>", saw reuse of working copies when unleased resources were avilable.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2015
Differential Revision: https://secure.phabricator.com/D4216
Summary:
This is very preliminary and doesn't actually do anything useful. In theory, it uses Drydock to check out a working copy and run tests. In practice, it's not actually capable of running any of our tests (because of complicated interdependency stuff), but does check out a working copy and //try// to run tests there.
Adds various sorts of utility methods to various things as well.
Test Plan: Ran `reparse.php --harbormaster --trace <commit>`, observed attempt to run tests via Drydock.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2015, T1049
Differential Revision: https://secure.phabricator.com/D4215
Summary:
This feature isn't necessarily intentional, but might as well make it work better.
Fixes T2201.
Test Plan: Verified that ##[[link | `name`]]## renders in blue.
Reviewers: chad, btrahan, vrana
Reviewed By: chad
CC: aran
Maniphest Tasks: T2201
Differential Revision: https://secure.phabricator.com/D4217
Summary: I plan to use this in Arcanist.
Test Plan:
$ echo '{}' | arc call-conduit x
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4192
Summary: Wide content is currently allowed to push too far to the right, overlapping icons. Prevent it from doing so.
Test Plan:
Before:
{F27906}
After:
{F27907}
After, Mobile:
{F27908}
Reviewers: chad, codeblock, btrahan
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D4212
Summary:
- If a Paste has very long lines, we show far too much text in the summary, because we wrap the lines. Instead, overflow them.
- If a Paste has very long unbroken lines (MMMMM...), they extend past the page. Instead, add a scrollable container.
Test Plan:
{F27868}
{F27869}
{F27870}
Reviewers: btrahan, codeblock, chad
Reviewed By: codeblock
CC: aran
Differential Revision: https://secure.phabricator.com/D4211
Summary:
Modernizes file uploads. In particular:
- Adds a mobile menu, with an "Upload File" item.
- Adds crumbs to the list view, detail view and upload view.
- Adds "Upload File" action to crumbs.
- Moves upload file to a separate page.
- Removes the combined upload file + recent files page.
- Makes upload file use a normal file control by default (works on mobile).
- Home page, file list and file upload page are now global drop targets which accept files dropped anywhere on them. Dragging a file into the window shows a mask and an instructional message.
- User education on this is a little weak but I think that's a big can of worms?
- Fixes a bug where dropping multiple files into a Remarkup text area produced bad results (resolves T2190).
T879 is related, although it's specifically about Maniphest. I've declined to make global drop targets yet there because there are multiple drop targets on the page with different meanings. That UI needs updating in general.
@chad, do we have an "upload" icon (counterpart to "download")?
Test Plan: Uploaded files in Maniphest, Differential, Files, and from Home. Dragged and dropped multiple files into Differential. Used crumbs, mobile.
Reviewers: chad, btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2190
Differential Revision: https://secure.phabricator.com/D4200
Summary:
D4188 adds a preview of Paste contents to the list, but gets slow for large lists if you have Pygments installed since it has to spend ~200ms/item highlighting them. Instead, cache the highlighted output.
- Adds a Paste highlighting cache. This uses RemarkupCache because it has automatic GC and we don't have a more generalized cache for the moment.
- Uses the Paste cache on paste list and paste detail.
- Adds a little padding to the summary.
- Adds "..." if there's more content.
- Adds line count and language, if available. These are hidden on Mobile.
- Nothing actually uses needRawContent() but I left it there since it doesn't hurt anything and is used internally (I thought the detail view did, but it uses the file content directly, and must for security reasons).
Test Plan:
{F27710}
- Profiled paste list, saw good performance and few service calls.
- Viewed paste.
- Viewed raw paste content.
Reviewers: codeblock, btrahan, chad
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D4204
Summary:
Use modern elements for file detail view.
For the "Technical Details", maybe we should implement a disclosure triangle element? I'd guess there are other cases we could use it.
This makes two small practical changes:
- We show "Delete File" even if you can't delete it; I'm going to align this properly with CAN_EDIT shortly.
- We no longer show the feature discovery hint about using "arc download".
Test Plan:
{F27179}
{F27180}
Reviewers: chad, btrahan
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D4199
Summary: Update to new UI stuff, prepare for mobile.
Test Plan: {F27167}
Reviewers: btrahan, chad
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D4198
Summary:
This adds a "body" field to `PhabricatorObjectItemView` which lets you optionally
add more information to the list view. It then uses this new field to show
samples of pastes in the paste list view.
Test Plan:
{F27147}
{F27148}
Reviewers: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4188
Summary: Include task ID and class when raising this exception. I took a brief stab at doing this generically, but (a) we specifically raise this exception outside of normal try/catch because we can't follow normal recovery rules for it and (b) we don't have a reasonable PhutilProxyException or similar right now which would preserve stack traces, and don't have builtin exception nesting support until PHP 5.3.
Test Plan: Faked this exception, verified we get more information in the logs.
Reviewers: btrahan, vrana
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2193
Differential Revision: https://secure.phabricator.com/D4205
Summary: Most cells in Diffusion were aligning text to the top, and the sigil height is fixed. This lines all up on center.
Test Plan: Tested diffusion with new cell alignment, clicked around to other tables. Looks good in Chrome.
Reviewers: epriestley, asherkin, btrahan
Reviewed By: btrahan
CC: aran, Korvin
Maniphest Tasks: T2189
Differential Revision: https://secure.phabricator.com/D4210
Summary: This tones down the glow and highlight for a cleaner hover experience.
Test Plan: chrome, firefox
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4207
Summary:
Add a `setViewer()` so that PHID loading doesn't throw.
Also remove a period from a button because none of the others have one.
Test Plan: Saved a comment successfully.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4202
Summary: I think I forgot this on my icon update
Test Plan: run celerity
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4203
Summary: Added 6px to the left and right of each TD on the revision history table.
Test Plan: Used Chrome and a table with many revisions.
Reviewers: vrana, epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4197
Summary:
- Fixes a bug where dragging a flexible left nav and then pressing "f" to hide it didn't properly reset the content panel's left margin.
- Fixes a bug where D4185 removed "white-space: nowrap;" accidentally (from @asherkin)
Test Plan:
- Dragged bar, then pressed "f".
- Can't repro the nowrap thing for some reason.
iiam
Reviewers: asherkin, vrana
Reviewed By: vrana
CC: aran
Differential Revision: https://secure.phabricator.com/D4194
Summary: Permit the forcible release of Drydock leases. The implementation isn't very exciting for now.
Test Plan: Released leases via web and CLI.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2015
Differential Revision: https://secure.phabricator.com/D4181
Summary:
Fixes glitches in the side nav. Resolves T1828. Resolves T2156.
- Elastic scrolling (T2156): in Safari on OSX, using a scroll touch on the trackpad to scroll up past the top of the document caused newer-style side menus to scroll down, leaving a visible whitespace bar.
- Whitespace glitch: Particularly in Safari, scrolling down the document quickly from the top caused the top menu to scroll away before the side menu rose to meet it. Use a fixed background color bar that extends under the menu so this doesn't happen.
- Use of "!important": use CSS better so we don't need to "!important" things.
- Dark Console (T1828): Instead of hard-coding the top position, determine it dynamically by looking at where the content is. This also fixes the menu overlapping with the red "there are errors on this page" development bar.
- General "fixed" glitchiness: don't use fixed-position for menu content other than flexible (draggable) menus.
Test Plan:
- Viewed and scrolled menus in Paste. Opened and closed DarkConsole. Switched devices.
- Viewed and scrolled flexible menus in Differential and Diffusion. Opened and closed DarkConsole. Switched devices.
Reviewers: vrana, chad, btrahan
Reviewed By: vrana
CC: aran
Maniphest Tasks: T1828, T2156
Differential Revision: https://secure.phabricator.com/D4185
Summary: Add a WIKI block to the switch in PhabricatorObjectHandleData->loadObjects().
Test Plan: Went to /feed/ and saw my wiki edit stories.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4180
Summary: See discussion in T2014. Aligns this element more closely with @chad's `frame_v3.psd` mock, and implements the icon/label element. Removes "details".
Test Plan: {F27062} {F27063} {F27064} {F27065}
Reviewers: btrahan, chad
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2014
Differential Revision: https://secure.phabricator.com/D4179
Summary: most awesome is that differential-primary-pane no longer has a place in diffusion. less awesome is fixing the zebra striping on differential "Local Commits" view and making the font size of one of the table headers match the others.
Test Plan: looks good!
Reviewers: epriestley, chad
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4177
Summary: we don't always have a diff so instead set an explicit title in the controller.
Test Plan: no more fatals. grepped carefully for every call site and tested them all
Summary: basically made the header elements for the individual panes and cleaned up the panes to make it look okay. tried to copy colors from @chad 's mocks.
Test Plan: looks good to me
Reviewers: epriestley, chad
Reviewed By: chad
CC: aran, Korvin
Maniphest Tasks: T2004
Differential Revision: https://secure.phabricator.com/D4174
Summary:
If something that doesn't belong to any field appears in the commit message
below the differential revision field, it gets included as part of the
value for the field, which can mess up parsing.
Test Plan:
called differential.parsecommitmessage on a commit whose differential
revision field wasn't being parsed earlier (it had a line of dashes two
lines below the Differential Revision: line).
Reviewers: epriestley, vrana
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4171
Summary:
Right now, Drydock gives out multiple leases to the same working copy and gives out leases to working copies with repository "P" in them when the user requested some other repository.
Add two callbacks:
- `canAllocateLease()` - allows a blueprint to reject a lease on a resource because of a fundamental incompatibility, like "it's a working copy with Phabricator in it, but the lease wants a working copy with Javelin in it".
- `shouldAllocateLease()` - allows a blueprint to reject a lease on a resource because of resource limits, like "only one active lease can own a working copy at a time".
Also cleaned up various other things.
Test Plan:
After implementing the callbacks, Drydock has the correct behavior:
- It gives multiple leases on `localhost`, but only one lease per working-copy resource.
- It does not grant leases on resources with repository X to requests for repository Y.
Ran `bin/drydock lease --type working-copy --repositoryID 12` and similar repeatedly and verified results in the web console.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2015
Differential Revision: https://secure.phabricator.com/D4166
Summary:
- Remove "Diffusion" crumb (redundant with application icon)
- Put "All Repositories" in its place on the landing page.
- Render the repository crumb as "rX" instead of "X Repository".
Test Plan: Looked at various Diffusion pages.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D4173
Summary:
upgrades are CrumbsView, HeaderView, PropertyListView, and ActionListView. I had to modify CrumbsView stuff a bit to handle the "advanced" diffusion crumbs.
Quirks fixed include making file tree view show up in diffusion, the page not have extra space when the file tree is hidden, links no longer breaking once you visit files (since without the change the files always got "/" appended and thus 404'd), and a differential quirk where it read "next step:" and that colon is a no no,
Test Plan: played around in diffusion and differential
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin, chad
Maniphest Tasks: T2048, T2178
Differential Revision: https://secure.phabricator.com/D4169
Summary:
Clicking "show details" of a task description change in Maniphest currently throws an exception about the markup engine.
Since we don't actually need the engine an alternate fix would be "if ($this->markupEngine) { $renderer->setMarkupEngine($this->markupEngine); }" but we have one at the ready so just provide it. This should become part of the Transactions stuff anyway.
Test Plan: Clicked "show details".
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D4167
Summary:
This adds a configuration option for repositories to be marked as managed
by phabricator, which is then used by the pullLocal daemon to try to recover
from some errors it runs into.
Test Plan: Set a bogus uri for a repo, saw that the pullLocal daemon fixed it.
Reviewers: epriestley, vrana
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D3680
Summary: Changes the tan table with the girly blue header color.
Test Plan: Chrome
Reviewers: vrana, epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2176
Differential Revision: https://secure.phabricator.com/D4164
Summary: Show attributes on the view pages.
Test Plan: {F26985} {F26986}
Reviewers: btrahan, chad
Reviewed By: chad
CC: aran
Maniphest Tasks: T2015
Differential Revision: https://secure.phabricator.com/D4165
Summary: wanted to play with some policy stuff as its been a bit. Turns out you can't edit questions so this is very silly "so long as you are a user you can view it" policy. also sorry if you have a diff or twelve out for this in your sandbox(es).
Test Plan: loaded up ponder and clicked about
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2113
Differential Revision: https://secure.phabricator.com/D4163
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: Currently, AphrontProxyResponse is expected to build a string. This prevents some response types (like Dialog) from being proxied, because they have special rules. Instead, make proxy responses reduce into a non-proxied response so it's possible to proxy any type of response and hit all the normal rules for it.
Test Plan: Built a proxied DialogResponse on top of this.
Reviewers: btrahan, vrana
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2104, T912
Differential Revision: https://secure.phabricator.com/D4159
Summary:
For transaction interfaces, I want to prompt the user when they take an action that has no effect, e.g.:
Action Has No Effect
You can not close this task, because someone else has already closed it.
Do you want to post your comment anyway?
[Cancel] [Post Comment]
We already do this for Differential, but it's all hard-coded. T912 is an open task for fixing this for Maniphest.
To do this in a general way, I want to embed the entire request in the dialog as hidden inputs, then add a "__continue__" key and resubmit the form. The endpoint will read this key the second time through and apply what effects it can (e.g., just post a comment).
This adds a mechanism for getting all the request data, minus "magic" like __dialog__ and __csrf__. We need to jump through some hoops because of how PHP encodes arrays.
Test Plan: Ran unit tests, built "no effect" dialogs on top of this.
Reviewers: btrahan, vrana
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T912, T2104
Differential Revision: https://secure.phabricator.com/D4158
Summary: pull out some more stuff from the TwoUp renderer that's generically useful. also clean up $xhp variable and add a get method for the $coverage. Finally, fix T2177 while I'm in here.
Test Plan: played around with Differential. Also opened things up in Firefox to verify T2177.
Reviewers: epriestley, vrana
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2009, T2177
Differential Revision: https://secure.phabricator.com/D4161
Summary: I thought I'd already implemented this, but hadn't. Implement a "USER" policy -- a USER phid means only that user has the capability.
Test Plan: Looked at macros as a user other than the comment owner.
Reviewers: btrahan, vrana
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D4155
Summary: D4153 made these render with newlines between items; use commas instead.
Test Plan: {F26950}
Reviewers: btrahan, chad, vrana
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D4162
Summary:
Git adds it automatically.
I don't like this solution much because there could be other unknown fields appended to the end of the commit message which will break parsing.
Test Plan:
Reparsed commit ending with:
> Differential Revision: ...
> Conflicts: ...
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4157
Summary: actions are still a bit messy - unsatisfactory icons (T2013 will help!)
Test Plan: viewed diffs - they look good
Reviewers: epriestley, vrana
Reviewed By: epriestley
CC: chad, aran, Korvin
Maniphest Tasks: T2007
Differential Revision: https://secure.phabricator.com/D3904
Summary:
For text like "MMM", make the right parts of the element scroll.
Also fixed a couple of 1px issues here and there.
Test Plan: Added, viewed UIExamples.
Reviewers: chad, btrahan
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D4152
Summary:
When possible, render application transactions via Ajax. Instead of reloading the page when the response returns, append new transactions to the transaction view.
Scroll the window to the new transactions, animate them in, and clear the form to make this interaction feel reasonable.
When editing transactions, fade them in but do not scroll to them (i.e., don't disrupt the user's position).
Test Plan: Edited and appended transactions via Ajax. Observed fade in animations and scroll behavior. Clicked anchors to verify proper anchor accounting.
Reviewers: vrana, btrahan, chad
Reviewed By: vrana
CC: aran
Maniphest Tasks: T1960
Differential Revision: https://secure.phabricator.com/D4151
Summary: When possible, replace the edited or deleted transaction inline using Ajax.
Test Plan: Repeatedly edited and deleted transactions. Clicked their anchors to verify anchors were correctly preserved.
Reviewers: vrana, btrahan, chad
Reviewed By: vrana
CC: aran
Maniphest Tasks: T1082
Differential Revision: https://secure.phabricator.com/D4150
Summary:
Allows you to edit or delete comments in appplications which support ApplicationTransactions.
UI/UX stuff:
- The dialogs are rough but I want to do a dialog design pass more generally, @chad has some mocks.
- When you add new mentions via edit, they don't currently count as mentions. I'm not sure what I want to do about this.
- When you edit or delete a comment, we do not publish any notifications about it. I think this is reasonable.
- I didn't separate "delete" out versus "edit"; I assume it will be reasonably intuitive that deleting all the text deletes effectively deletes the comment. I also want to discourage deletion somewhat (we still show the transaction, just show that the comment has been deleted).
Test Plan:
Transaction view, note "Edit" and "Edited" links:
{F26914}
Edit view, has some design issues but I want to do a pass on dialogs in general:
{F26915}
History view:
{F26913}
Reviewers: vrana, btrahan, chad
Reviewed By: vrana
CC: aran
Maniphest Tasks: T1082
Differential Revision: https://secure.phabricator.com/D4149
Summary: Aligns more styles to the `diff_full_view.png` mock.
Test Plan: {F26859}
Reviewers: chad, btrahan
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D4142
Summary:
Suhosin has about 50 options for filtering input variables, doucmented here:
http://www.hardened-php.net/suhosin/configuration.html
The default behavior of Suhosin is to drop the variable entirely if it violates any of the rules, then continue with the request. It doesn't affect 'php://input' and doesn't drop other variables, so it evades existing detection, and we can't figure out that it's happened at runtime. We could add blanket checks (Suhosin enabled + suhosin.filter.action set to nothing means this may happen, and will be undetectable if it does happen) but can't tailor a check or recovery to this specific problem.
Instead, raise a better error in the specific case where we encounter this, which is Conduit calls of "arc diff" of files over 1MB (the default POST limit). In these cases, Suhosin drops the variable entirely. If there is no 'params', scream. We never encounter this case normall (`arc`, including `arc call-conduit`, always sends this parameter) although other clients might omit it. The only exception is the web console with `conduit.ping`, which submits nothing; make it submit something so it keeps working.
See also https://github.com/facebook/phabricator/issues/233#issuecomment-11186074
Test Plan: Brought up a Debian + Suhosin box, verified the behavior of Suhosin, made requests with and without 'params'.
Reviewers: btrahan, vrana
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D4144
Summary: Publish feed stories, including from Pholio. Actual stories are somewhat garbage but it's all display-time; I'm going to do a pass on feed in general.
Test Plan: {F26832}
Reviewers: btrahan, chad, vrana
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2104
Differential Revision: https://secure.phabricator.com/D4140
Summary:
- Adds mail support to the generic transaction construct.
- Restores mail support to Pholio (now much improved; the mails are actually useful).
Test Plan: Updated a Pholio mock, got mail.
Reviewers: btrahan, chad, vrana
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2104
Differential Revision: https://secure.phabricator.com/D4139
Summary: I got rid of the "#4" and just linked the timestamps.
Test Plan: {F26826}
Reviewers: chad
Reviewed By: chad
CC: aran
Maniphest Tasks: T2104
Differential Revision: https://secure.phabricator.com/D4138
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: This is to reduce number of calls from Arcanist.
Test Plan: Called it from web interface.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4146
Summary: Minor issue from D4117, user doesn't get passed down so inline comments are missing their "reply" link.
Test Plan: Looked at Differential, saw reply link.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D4145
Summary: The dark highlight was bothering me, I'm assuming it probably bothered others but hadn't come up yet.
Test Plan: Hover reload hover reload hover.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4143
Summary:
Currently, code blocks inside Phabricator remarkup are constrained to ~80col. There's no technical or design reason for this -- it looks and works fine without the restriction. Join the new world of fluid widths.
(Note that //inline// comments can get a bit whacky in some cases with large code blocks, but we have no restrictions there currently, so this doesn't break anything.)
Test Plan: {F26814}
Reviewers: btrahan, chad
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2005
Differential Revision: https://secure.phabricator.com/D4135
Summary:
Support SMTP as the mailer and user could turn on SMTP authentication if needed.
Import PHPMailer as PHPMailerLite doesn't support SMTP.
Make class PhabricatorMailImplementationPHPMailerAdapter final.
Test Plan: N/A
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2139
Differential Revision: https://secure.phabricator.com/D4063
Summary:
D4117 (which is otherwise awesome :)) requires you to `setMarkupEngine()` and
Phriction's diff rendering wasn't changed to call that with a
`PhabricatorMarkupEngine`.
Test Plan: Went to a Phriction diff page and saw it render correctly.
Reviewers: epriestley, btrahan, vrana
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4132
Summary: This updates spacing and colors on the calendar page.
Test Plan: View calendar, make an event, view it again.
Reviewers: epriestley, btrahan
Reviewed By: btrahan
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4130
Summary: Adds a darker hover color for mousing over the logo.
Test Plan: Mouse over logo, feel the gentle hover wash over me like a light spring rain.
Reviewers: epriestley, btrahan
Reviewed By: btrahan
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4128
Summary: Adds little arrows in CSS to the tooltipcs.
Test Plan: Tested UIExamples and Remarkup Box
Reviewers: epriestley, btrahan
Reviewed By: btrahan
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4126
Summary: we need to render left and right* classes as appropriate, plus colspan for the right
Test Plan: made inline comments and it was no longer borked
Reviewers: vrana, epriestley, chad
Reviewed By: chad
CC: aran, Korvin
Maniphest Tasks: T2005
Differential Revision: https://secure.phabricator.com/D4131
Summary:
- Align the icon and the logo
- Have text links show underline on hover
Test Plan: Viewed the fruits of my efforts
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4125
Summary: I think we've sorted out enough of the problems with these to turn them on for everyone. The real-time component remains configuration-dependent.
Test Plan: Turned off "notification.enabled", still saw notifications.
Reviewers: btrahan, chad
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D4120
Summary:
- Gets about 25% of the way toward @chad's notification mocks.
- YES: Hover states, entire notification is a click target, border, header, footer.
- NO: Profile pictures (lazy), timestamps (want to refactor time code before introducing a new formatting style), app icons (they'd look funny without timestamps I think)
- Deletes some old files.
- Mostly trying to get this good enough to turn on by default.
Test Plan: Looked at notifications. Clicked some notifications.
Reviewers: chad, btrahan
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D4119
Summary:
Currently we have two different feed story classes, one for notifications and one for feed stories. However, we never actually do anything different with them -- the notification is always the same as the feed story, just shown differently. Delete the notification special case to reduce the amount of code we have supporting feed and notifications.
This is a precursor to @chad's notification designs.
Test Plan: Viewed notifications and feed, saw exactly the same result before and after the patch (but less, simpler code).
Reviewers: btrahan
Reviewed By: btrahan
CC: chad, aran
Differential Revision: https://secure.phabricator.com/D4114
Summary:
basically did my darnedest to pull out a TwoUp rendering view. Made a base class for the rendering views with "old" and "new" terminology rather than "left" and "right.
Future revisions will finish cleaning up the terminology within the DifferentialChangesetParser itself and more of the ideas within T2009.
Test Plan: been playing with differential all day
Reviewers: epriestley
Reviewed By: epriestley
CC: vrana, chad, aran, Korvin
Maniphest Tasks: T2009
Differential Revision: https://secure.phabricator.com/D4117
Summary: we were catching a specific exception; just catch all exceptions
Test Plan: viewed repository tool home page
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2155
Differential Revision: https://secure.phabricator.com/D4118
Summary: inline comments eat up all 3 tds and no code coverage should be shown.
Test Plan: verified in FIREFOX that inline comments looked good
Reviewers: vrana, epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2005
Differential Revision: https://secure.phabricator.com/D4115
Summary:
- Remove unused CSS rules and non-sprite images.
- Sprite the logo.
Test Plan: Looked at site, looked good.
Reviewers: chad
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D4113
Summary: Adds very basic crumbs to Differential, to prevent regression when we drop the application menu. I'll do a more proper pass at this but want to unblock landing the commit sequence for all this stuff.
Test Plan: Looked at detail view and list view, saw crumbs, clicked them.
Reviewers: chad
Reviewed By: chad
CC: aran
Maniphest Tasks: T1960
Differential Revision: https://secure.phabricator.com/D4111
Summary:
Issues here:
- Need an application-sized "eye", or a "home" icon for "Phabricator Home".
- Some of the "apps_lb_2x" sliced images are the "_dark_" versions, not the light versions.
- If you slice an application-sized "logout" (power off) icon and application-sized "help" (questionmark in circle) icon I can replace the current menu icons and nearly get rid of "autosprite".
- To replace the icons on /applications/, the non-retina size is "4x", so we'd need "8x" for retina. Alternatively I can reduce the icon sizes by 50%.
- The "Help", "Settings" and "Logout" items currently have a "glowing" hover state, which needs a variant (or we can drop it).
- The /applications/ icons have a white hover state (or we can drop it).
- The 1x application (14x14) icons aren't used anywhere right now, should they be? Maybe in the feed in the future, etc?
- The "apps-2x" and "apps-large" sheets are the same image, but getting them to actually use the same file is a bit tricky, so I just left them separate for now.
Test Plan:
{F26698}
{F26699}
Reviewers: chad
Reviewed By: chad
CC: aran
Maniphest Tasks: T1960
Differential Revision: https://secure.phabricator.com/D4108
Summary: Add a ".device" rule which means "phone or tablet". Simplify about 5000 rules which were written ".device-phone X, device-tablet X { ... }".
Test Plan: Browsed the site a bit without incident.
Reviewers: chad
Reviewed By: chad
CC: aran
Maniphest Tasks: T1960
Differential Revision: https://secure.phabricator.com/D4103
Summary: Do we have an icon with 2x for the right menu?
Test Plan: {F26590}
Reviewers: chad
Reviewed By: chad
CC: aran
Maniphest Tasks: T1960
Differential Revision: https://secure.phabricator.com/D4101
Summary:
This doesn't lay in everything, but:
- Break the buttons gradient apart into components and rebuild it (along with other gradients) into a single gradient sprite (possible after {D4099}).
- Use the sliced gradient for the crumbs background.
- Use the sliced image for the crumb divider.
- Adds the black/white app sheets, but I'm not generating them quite yet.
Test Plan: {F26537} {F26540}
Reviewers: chad, btrahan
Reviewed By: chad
CC: aran
Maniphest Tasks: T1960
Differential Revision: https://secure.phabricator.com/D4100
Summary:
Add a basic breadcrumbs element, and implement it in Paste.
This needs some polish but is most of the way there.
Test Plan:
{F26443}
{F26444}
{F26445}
(This element is not visible on devices.)
Reviewers: chad
Reviewed By: chad
CC: aran, btrahan
Maniphest Tasks: T1960
Differential Revision: https://secure.phabricator.com/D4087
Summary:
Adds a right-hand-side application menu, based roughly on `frame_v3.png`.
This has the same icon as the left menu until we get real design in, but is functionally reasonable.
Test Plan: {F26170} {F26169}
Reviewers: chad
Reviewed By: chad
CC: aran
Maniphest Tasks: T1960
Differential Revision: https://secure.phabricator.com/D4061
Summary:
As per discussion, this primes the existing mobile menu / menu button for "phabricator" and "application" menus.
Design here is very rough, I'm just trying to get everything laid in functionally first. It's based on `frame_v3.png` but missing a lot of touches.
Test Plan:
{F26143}
{F26144}
Reviewers: chad
Reviewed By: chad
CC: aran
Maniphest Tasks: T1960
Differential Revision: https://secure.phabricator.com/D4058
Summary:
- Adds `PhabricatorMenuItemView` which is a non-hacky object representing a single menu item.
- Adds `PhabricatorMenuView`, a collection of items.
- Deletes some busted/old interfaces full of garbage nonsense.
- Merges menu item styles from `aphront-side-nav-view-css` and `phabricator-nav-view-css`. These are old-style and new-style rules which got partially updated recently.
- The new-style menus have a darker background (#ececec) than the old-style menus (#f7f7f7) so some of the highlight/hover colors weren't visible. I shuffled them around but something or other might need further adjustment.
Test Plan: looked at every menu I could
Reviewers: chad
Reviewed By: chad
CC: aran
Maniphest Tasks: T1960
Differential Revision: https://secure.phabricator.com/D4036
Summary: After D4034 there are no AphrontSideNavView callsites outside of AphrontSideNavFilterView.
Test Plan: Looked at about a million side navs.
Reviewers: chad
Reviewed By: chad
CC: aran
Maniphest Tasks: T1960
Differential Revision: https://secure.phabricator.com/D4035
Summary:
AphrontSideNavView is an old class which required you to do a lot of work; it was obsoleted by AphrontSideNavFilterView. Remove all direct callsites so I can clean it up.
This is a precursor to letting me render a filter menu as a dropdown menu for T1960.
Test Plan:
Examined each interface for correct filter construction and selection:
- Browsed Diffusion
- Browsed Differential
- Browsed Files
- Browsed Slowvote
- Browsed Phriction
- Browsed repo edit interface
Grepped for `AphrontSideNavView`. The only remaining instances are in `AphrontSideNavView` itself and `AphrontSideNavFilterView` (which currently uses it).
Reviewers: chad
Reviewed By: chad
CC: aran
Maniphest Tasks: T1960
Differential Revision: https://secure.phabricator.com/D4034
Summary:
Toss this completely as per discussion elsewhere. Basically it doesn't feel as useful as we imagined it would, and breadcrumbs from T1960 will replace the primary useful part (navigating up).
There's some more cleanup to do but I'll hit that in the next few diffs.
Closes T1828 as wontfix.
Test Plan: Viewed app + local, app-without-local interfaces. Saw no app menus.
Reviewers: chad
Reviewed By: chad
CC: aran, vrana
Maniphest Tasks: T1828, T1960
Differential Revision: https://secure.phabricator.com/D4033
Summary: when we had a change that had new data and uncommitted changes the colspan could get off. make sure to only decrement the colspan if we are actually on a new line
Test Plan: http://phabricator.dev/rP1a3bf098ae4ab4f8add4af744a6b93a257851fb0 now looks good on Firefox
Reviewers: vrana, epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2005
Differential Revision: https://secure.phabricator.com/D4109
Summary: See D3977, a terrible diff where I made a huge messs.
Test Plan: Ran `reparse.php --message --trace <revision>`, verified correct identification of author.
Reviewers: edward, vrana
Reviewed By: edward
CC: aran
Differential Revision: https://secure.phabricator.com/D4104
Summary:
assume at least 360px for a given code pane. that's about when the comment box starts fighting back anyway. we'll use the yet-to-be-built one page render for the narrow viewport cases.
This address the cases as laid out in T2005. It fails the "MMMMM" case pretty horribly. However, if there is a space it works just fine and presumably folks are stretching out their windows on big glorious monitors at 160 characters wide or whatever.
Re-factored things just a tad but figure I'll take a nice big chunk of "renderer" to move forward T2009
Test Plan: looked at all sorts of funky diffs
Reviewers: epriestley
Reviewed By: epriestley
CC: chad, aran, Korvin
Maniphest Tasks: T2005
Differential Revision: https://secure.phabricator.com/D4083
Summary: Happens on the end of hunk.
Test Plan:
$ ./reparse.php
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4091
Summary: See discussion in T1544. This has been obsoleted by simpler/better mechanisms.
Test Plan: Edited a repository; ran a parse task.
Reviewers: edward
Reviewed By: edward
CC: aran
Maniphest Tasks: T1544
Differential Revision: https://secure.phabricator.com/D3977
Summary:
I got to learn a lot about SSL/TLS today.
Try to make it easier to troubleshoot SSL issues.
Test Plan: Read document.
Reviewers: btrahan, vrana
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2132
Differential Revision: https://secure.phabricator.com/D4079
Summary: In retrospect this is probably mostly obvious from the UI.
Test Plan: {F26314}
Reviewers: vrana
Reviewed By: vrana
CC: aran
Maniphest Tasks: T2095
Differential Revision: https://secure.phabricator.com/D4075
Summary: `addDetail()` takes HTML because we have links there fairly often. :/ This design is iffy.
Test Plan: Reloaded `/calendar/status/`, verified no XSS.
Reviewers: btrahan, vrana
Reviewed By: vrana
CC: aran
Maniphest Tasks: T139
Differential Revision: https://secure.phabricator.com/D4074
Summary: I didn't catch this issue at D3986 because we don't have `DifferentialManiphestTasksFieldSpecification` in field selector.
Test Plan:
Added `DifferentialManiphestTasksFieldSpecification` too field selector.
Wrote `Refs T4` and not filled `T4` to Maniphest Tasks field.
Reviewers: epriestley, 20after4
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T945
Differential Revision: https://secure.phabricator.com/D4073
Summary:
I want to add search per owner, this is a prerequisity for it.
There's no link to this page yet, I didn't find a good place for it.
Test Plan: Displayed it, clicked around.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin, scottmac
Maniphest Tasks: T2038
Differential Revision: https://secure.phabricator.com/D4067
Summary: D4027 introduced this along with associated awesomeness.
Test Plan: viewed uiexample and saw some actions. also no more errors.
Reviewers: epriestley
Reviewed By: epriestley
CC: chad, aran, Korvin
Differential Revision: https://secure.phabricator.com/D4066
Summary: make sure we only print the fancy tool tip thing if we have all the data we need. (fixes T2136). additionally, default $branches to array() to prevent errors from reset on null.
Test Plan: made a checkin for a mercurial repo with user "foo@bar" with no branch. verified name showed up in all views. also noted on commit detail view there was no more error about reset() on null for $branches.
Reviewers: epriestley
Reviewed By: epriestley
CC: chad, aran, Korvin
Maniphest Tasks: T2136
Differential Revision: https://secure.phabricator.com/D4065
Summary: The construct `count(x == 0)` should be `count(x) == 0`. This causes a concrete problem because `count(0)` is 1.
Test Plan: eyeballed it~
Reviewers: btrahan, vrana, klimek
Reviewed By: klimek
CC: aran
Maniphest Tasks: T1643
Differential Revision: https://secure.phabricator.com/D4060
Summary:
Users are used to this feature from comments.
Provide it also in title, summary and test plan.
It adds the users to CC only on creating the revision to avoid cases like:
"I mentioned this user but now I want to remove him from CC" or he unsubscribes.
Test Plan: Wrote `@epriestley` to summary.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4050
Summary:
This breaks Herald pretty badly too. We could restore it selectively; which buttons was it intended to affect?
{F26039}
Test Plan: Herald looks more reasonable now.
Reviewers: chad
Reviewed By: chad
CC: aran
Maniphest Tasks: T2090
Differential Revision: https://secure.phabricator.com/D4046
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: Switch to the final versions of these
Test Plan: Will add screenshots...
Reviewers: chad
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D4032
Summary: This is missing a lot of features, but technically allows working copy allocation.
Test Plan: Ran `drydock lease --type working-copy --attributes repositoryID=12`, got a working copy of Phabricator allocated on disk.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2015
Differential Revision: https://secure.phabricator.com/D3999
Summary: This does nothing fancy, just closes the resource and releases/breaks leases. They'll get cleaned up in some to-be-written GC process.
Test Plan: Closed resources from web UI and CLI.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2015
Differential Revision: https://secure.phabricator.com/D3998
Summary:
Builds out most of the non-hover-stuff from `overview-hovercards.png`. Things I didn't build:
- Tokens (I like them a lot but don't want to scope creep)
- Functions (backend mess / future work)
- Icons for tags.
- Tags with pointy ends and holes in them (an earlier mock had this I think but they're gone on final)
- The cyaney color for "Sporadic" since I just noticed it while typing this up.
Test Plan: Looked at UIExample page.
Reviewers: chad
Reviewed By: chad
CC: aran
Maniphest Tasks: T2089
Differential Revision: https://secure.phabricator.com/D4029
Summary:
- Remove `app_audit.png`, which is now part of the sprite (it was held back transitionally to make sure things worked).
- Remove `sprite.png`, which is the old button sprite. The only remaining use was on the home page, which I replaced with box shadows to achieve a similar effect. These buttons should probably go away at some point anyway.
Test Plan: {F25768}
Reviewers: chad
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D4028
Summary:
- The filesystem is now the authority for which sprites are available. If you add new icons, the generation process will pick them up.
- I broke out icon generation and added retina support. App icon generation still uses the old method.
- Update ActionList and RemarkupControl to use the new sheet.
- Use white icons on hover.
- Also fixed a couple of minor issues with some stuff in Firefox/Chrome.
Test Plan:
{F25750}
{F25751}
{F25752}
Reviewers: chad
Reviewed By: chad
CC: aran
Maniphest Tasks: T2013
Differential Revision: https://secure.phabricator.com/D4027
Summary:
- Since we'll never serve these directly, move them to resources/. This makes generating the Celerity map faster and reduces the size of the result map, since we don't need to analyze resources we'll never serve.
- Also Rename the 2x `subscribe-remove` to `subscribe-delete` since they were named inconsistently. Everything else is in good shape.
Test Plan: Generated sprites as per D4025
Reviewers: chad
Reviewed By: chad
CC: aran
Maniphest Tasks: T2013
Differential Revision: https://secure.phabricator.com/D4026
Summary:
When users middle click or command-click an image, we should open it in a new tab, not open a lightbox.
See https://github.com/facebook/phabricator/issues/234
Test Plan: Left, middle, and command-clicked a lightbox image.
Reviewers: vrana, chad, btrahan
Reviewed By: vrana
CC: aran
Differential Revision: https://secure.phabricator.com/D4020
