Summary: Ref T603. When you aren't allowed to take a top-level action (usually "Create Thing"), visually disable the action.
Test Plan: {F69596}
Reviewers: chad
Reviewed By: chad
CC: chad, aran, asherkin
Maniphest Tasks: T603
Differential Revision: https://secure.phabricator.com/D7318
Summary: Ref T603. When a user selects "Custom", we pop open the rules dialog and let them create a new rule or edit the existing rule.
Test Plan: Set some objects to have custom policies.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T603
Differential Revision: https://secure.phabricator.com/D7300
Summary:
Ref T603. Although I think the parenthetical is valuable when //setting// policies to make sure no one accidentally opens content up, it's super annoying in headers.
This makes headers say "Public". Everything else still says "Public (No Login Required)".
Test Plan: {F69469}
Reviewers: chad, btrahan
Reviewed By: chad
CC: aran
Maniphest Tasks: T603
Differential Revision: https://secure.phabricator.com/D7310
Summary: If we hit an exception very early, the request may not have a user yet. Don't assume it will.
Test Plan: Broke MySQL config on purpose to test some stuff, loaded Phabricator, got a nice error page instead of a super-fatal one.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D7286
Summary: Ref T603. Make this a little easier to use by highlighting the current value.
Test Plan: See screenshot.
Reviewers: chad, btrahan
Reviewed By: chad
CC: chad, aran
Maniphest Tasks: T603
Differential Revision: https://secure.phabricator.com/D7289
Summary: Ref T603. After thinking about this for a bit I can't really come up with anything better than what Facebook does, so I'm going to implement something similar for choosing custom policies. To start with, swap this over to a JS-driven dropdown.
Test Plan: See screenshot.
Reviewers: chad, btrahan
Reviewed By: btrahan
CC: chad, aran
Maniphest Tasks: T603
Differential Revision: https://secure.phabricator.com/D7285
Summary: This builds out and implements PHUIPropertyListView (container) and PHUIPropertyListItemView (section) as well as adding tabs.
Test Plan: Tested each page I edited with the exception of Releeph and Phortune, though those changes look ok to me diff wise. Updated examples page with tabs.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: Korvin, epriestley, aran
Differential Revision: https://secure.phabricator.com/D7283
Summary:
- Add an extra paginator at the top.
- Add a link to jump to the bottom (where the latest messages are).
- Align paginators with edge of content rather than the page.
Test Plan: Looked at the chatlog.
Reviewers: epriestley, chad, #blessed_reviewers
CC: chad, Korvin, epriestley, aran
Differential Revision: https://secure.phabricator.com/D7280
Summary: Ref T603. Currently, we hard-code defense against setting policies to "Public" in several places, and special case only the CAN_VIEW policy. In fact, other policies (like Default View) should also be able to be set to public. Instead of hard-coding this, move it to the capability definitions.
Test Plan: Set default view policy in Maniphest to "Public", created a task, verified default policy.
Reviewers: btrahan, asherkin
Reviewed By: asherkin
CC: asherkin, aran
Maniphest Tasks: T603
Differential Revision: https://secure.phabricator.com/D7276
Summary: Ref T603. When the user encounters an action which is controlled by a special policy rule in the application, make it easier for applications to show the user what policy controls the action and what the setting is. I took this about halfway before and left a TODO, but turn it into something more useful.
Test Plan: See screenshots.
Reviewers: btrahan, chad
Reviewed By: chad
CC: chad, aran
Maniphest Tasks: T603
Differential Revision: https://secure.phabricator.com/D7265
Summary: Adds the abilit to set a status color of warning or fail to navbar tab lists (for objectheaders)
Test Plan: uiexamples, photoshop
Reviewers: epriestley
Reviewed By: epriestley
CC: Korvin, epriestley, aran
Differential Revision: https://secure.phabricator.com/D7266
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:
Ref T603. Ref T1279. Further improves transaction and policy support for Herald.
- Instead of deleting rules (which wipes out history and can't be undone) allow them to be disabled.
- Track disables with transactions.
- Gate disables with policy controls.
- Show policy and status information in the headers.
- Show transaction history on rule detail screens.
- Remove the delete controller.
- Support disabled queries in the ApplicationSearch.
Test Plan:
- Enabled and disabled rules.
- Searched for enabled/disabled rules.
- Verified disabled rules don't activate.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T1279, T603
Differential Revision: https://secure.phabricator.com/D7247
Summary: This adds setActionList to PropertyListView and properly places it in an archaic HTML 1.0 table.
Test Plan: test layouts with actions really tall or properties really tall. Always see a full height border.
Reviewers: epriestley, btrahan
CC: Korvin, aran
Differential Revision: https://secure.phabricator.com/D7239
Summary: Ref T1279. Show separate sections for "Reviewers" and "Project Reviewers" (Differential) and for "Auditors" and "Package/Project Auditors" (Diffusion/Audit).
Test Plan:
- Looked at a commit. Saw separation.
- Looked at a revision. Saw separation.
Reviewers: chad, btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T1279
Differential Revision: https://secure.phabricator.com/D7233
Summary:
Ref T1279. Two changes to the search/query for Differential:
- "Reviewers" now accepts users and projects.
- "Responsible Users" now includes revisions where a project you are a member of is a reviewer.
Test Plan:
- Searched for project reviewers.
- Verified that the dashboard now shows reviews which I'm only part of via project membership.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T1279
Differential Revision: https://secure.phabricator.com/D7231
Summary:
Three changes here.
- Add `setActionList()`, and use that to set the action list.
- Add `setPropertyList()`, and use that to set the property list.
These will let us add some apropriate CSS so we can fix the border issue, and get rid of a bunch of goofy `.x + .y` selectors.
- Replace `addContent()` with `appendChild()`.
This is just a consistency thing; `AphrontView` already provides `appendChild()`, and `addContent()` did the same thing.
Test Plan:
- Viewed "All Config".
- Viewed a countdown.
- Viewed a revision (add comment, change list, table of contents, comment, local commits, open revisions affecting these files, update history).
- Viewed Diffusion (browse, change, history, repository, lint).
- Viewed Drydock (resource, lease).
- Viewed Files.
- Viewed Herald.
- Viewed Legalpad.
- Viewed macro (edit, edit audio, view).
- Viewed Maniphest.
- Viewed Applications.
- Viewed Paste.
- Viewed People.
- Viewed Phulux.
- Viewed Pholio.
- Viewed Phame (blog, post).
- Viewed Phortune (account, product).
- Viewed Ponder (questions, answers, comments).
- Viewed Releeph.
- Viewed Projects.
- Viewed Slowvote.
NOTE: Images in Files aren't on a black background anymore -- I assume that's on purpose?
NOTE: Some jankiness in Phortune, I'll clean that up when I get back to it. Not related to this diff.
Reviewers: chad
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D7174
Summary: This adds the 'PHUIObjectBox' to nearly every place that should get it. I need to comb through Diffusion a little more. I've left Differential mostly alone, but may decide to do it anyways this weekend. I'm sure I missed something else, but these are easy enough to update.
Test Plan: tested each new layout.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: Korvin, aran
Differential Revision: https://secure.phabricator.com/D7162
Summary:
Ref T603. If an install allows acccess by logged-out users, show search.
(A lot of the search typeahead results, although visible to the user, don't lead anywhere interesting right now. We can clean this up in the future.)
Test Plan: As a logged out user, searched for some stuff. It worked. Also, I only found results I could see, which is quite heartening.
Reviewers: btrahan, chad
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T603
Differential Revision: https://secure.phabricator.com/D7153
Summary:
Ref T603. Adds clarifying text which expands on policies and explains exceptions and rules. The goal is to provide an easy way for users to learn about special policy rules, like "task owners can always see a task".
This presentation might be a little aggressive. That's probably OK as we introduce policies, but something a little more tempered might be better down the road.
Test Plan: See screenshot.
Reviewers: btrahan, chad
Reviewed By: chad
CC: aran
Maniphest Tasks: T603
Differential Revision: https://secure.phabricator.com/D7150
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:
There may be times when we don't want lines in some embeded source
code to be highlighted on click, this adds that functionality.
Also use the functionalty in `PasteEmbedView`
Test Plan:
- View paste, make sure everything works.
- Embed paste in comment, make sure everything works apart from click hl
Reviewers: epriestley
Reviewed By: epriestley
CC: Korvin, aran
Maniphest Tasks: T3881
Differential Revision: https://secure.phabricator.com/D7111
Summary: TimelineView on Maniphest is often kind of hard to parse because related/simultaneous transactions aren't visually grouped. Allow grouping. I'm going to clean this up a little bit more.
Test Plan: See screenshot.
Reviewers: chad
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D7107
Summary: Adds status icons and colors to Maniphest and Differential. Also minor tweaks to them in hovercards. Probably some other stuff too.
Test Plan: Test many diff and task states.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: Korvin, aran
Differential Revision: https://secure.phabricator.com/D7098
Summary: Instead of rendering this in all callers, just pass the object into the header and let it figure out how to format it.
Test Plan: Looked at Legalpad, Paste, and Pholio.
Reviewers: chad, btrahan
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D7039
Summary:
Ref T418. This is fairly messy, but basically:
- Add a validation phase to TransactionEditor.
- Add a validation phase to CustomField.
- Bring it to StandardField.
- Add validation logic for the int field.
- Provide support in related classes.
Test Plan: See screenshot.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T418
Differential Revision: https://secure.phabricator.com/D7028
Summary: The adds the ability to set 'properties' such as state, privacy, due date to the header of objects.
Test Plan: Implemented in Paste, Pholio. Tested various states.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: Korvin, aran
Differential Revision: https://secure.phabricator.com/D7016
Summary: Pagers in Maniphest (and, to some degree, apps like Pholio) get lost a bit. Put them in a little box.
Test Plan: Looked at Maniphest and Pholio, pager was more obvious and less un-designed-looking.
Reviewers: chad, btrahan
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D6987
Summary: Restores any/all/user/exclude project filters to the new search.
Test Plan: Filtered stuff by projects.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D6951
Summary: Moves book view to use PHUIDocument, fix some other spacing issues.
Test Plan: Review a number of pages in Diviner.
Reviewers: epriestley
Reviewed By: epriestley
CC: Korvin, aran
Differential Revision: https://secure.phabricator.com/D6925
Summary: Adds plain support for object lists that just look like lists
Test Plan: review UIexamples and a number of other applications
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: Korvin, aran
Differential Revision: https://secure.phabricator.com/D6922
Summary: This is just renaming to PHUI (I like shorter text :)
Test Plan: reload workboard examples page, seems to not fatal and looks very appealing
Reviewers: epriestley
Reviewed By: epriestley
CC: Korvin, aran
Differential Revision: https://secure.phabricator.com/D6904
Summary: This moves the outer classes to the outmost div
Test Plan: Reviewed a few PHUIBox pages
Reviewers: epriestley
Reviewed By: epriestley
CC: Korvin, aran
Differential Revision: https://secure.phabricator.com/D6902
Summary: This adds a number of new styles for Diviner documentation. Not sure I've covered all the bases or wrote this in the most efficient manner, but passing it along now for early review before tightening everything up.
Test Plan: Review various class pages.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: Korvin, aran
Differential Revision: https://secure.phabricator.com/D6888
Summary: Also, don't try to load prefs for non-users.
Test Plan: toggle, save, look at something with a time. arc unit.
Reviewers: epriestley
Reviewed By: epriestley
CC: Korvin, aran
Differential Revision: https://secure.phabricator.com/D6796
Summary:
Fixes T3782. Two changes:
- Remove the "Chaos" mode, which wasn't as funny as I'd hoped and has had a good run.
- Fix "Order" (now "Fullscreen") mode in Conpherence. Best fix I could come up with is dropping the "position: fixed" on all parents while in the mode.
Test Plan: Used Fullscreen mode in Conpherence in Chrome.
Reviewers: chad
Reviewed By: chad
CC: aran
Maniphest Tasks: T3782
Differential Revision: https://secure.phabricator.com/D6844
Summary: Fixes T3486. I don't love how this looks -- maybe we could try different icons? Like white icons on a brighter red/yellow background?
Test Plan: {F56299}
Reviewers: chad, btrahan
Reviewed By: chad
CC: aran
Maniphest Tasks: T3486
Differential Revision: https://secure.phabricator.com/D6833
Summary: Adds the new gradient to document views
Test Plan: Tested multiple pages in my sandbox in Phriction, UIExamples.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: Korvin, aran
Differential Revision: https://secure.phabricator.com/D6827
Summary: Updates the pinboard layout with less shadow and more standard border colors.
Test Plan: Reload pinboard
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: Korvin, aran
Differential Revision: https://secure.phabricator.com/D6829
Summary: Some more callsites, let me know if you see others, I think think is 98% of them now.
Test Plan: tested each page
Reviewers: epriestley, btrahan
Reviewed By: btrahan
CC: Korvin, aran
Differential Revision: https://secure.phabricator.com/D6814
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: Depends on D6769, removes 'dust' and uses a similar color background.
Test Plan: Review colors in sandbox.
Reviewers: epriestley
Reviewed By: epriestley
CC: Korvin, aran
Differential Revision: https://secure.phabricator.com/D6772
Summary:
Ref T3721. Releeph currently attempts to implement a flexible, field-driven search for branches, but it's building all of its own infrastructure and it ends up heading down some weird paths. In particular, it loads **every** request and then makes calls into fields to filter them. It also tries to be very very general, which isn't really necessary (for example, I think it's reasonable for us to assume that we won't let you disable the "requestor" field).
ApplicationSearch and CustomField provide more scalable approaches to this problem; move search on top of them. The query still ends up doing some filtering in-process, but it's now far more limited in scope and can be denormalized later.
Test Plan: {F54304}
Reviewers: btrahan
Reviewed By: btrahan
CC: chad, aran
Maniphest Tasks: T3721
Differential Revision: https://secure.phabricator.com/D6758
Summary:
Ref T1702. Ref T3718. There are a couple of things going on here:
**PhabricatorCustomFieldList**: I added `PhabricatorCustomFieldList`, which is just a convenience class for dealing with lists of fields. Often, current field code does something like this inline in a Controller:
foreach ($fields as $field) {
// do some junk
}
Often, that junk has some slightly subtle implications. Move all of it to `$list->doSomeJunk()` methods (like `appendFieldsToForm()`, `loadFieldsFromStorage()`) to reduce code duplication and prevent errors. This additionally moves an existing list-convenience method there, out of `PhabricatorPropertyListView`.
**PhabricatorUserConfiguredCustomFieldStorage**: Adds `PhabricatorUserConfiguredCustomFieldStorage` for storing custom field data (like "ICQ Handle", "Phone Number", "Desk", "Favorite Flower", etc).
**Configuration-Driven Custom Fields**: Previously, I was thinking about doing these with interfaces, but as I thought about it more I started to dislike that approach. Instead, I built proxies into `PhabricatorCustomField`. Basically, this means that fields (like a custom, configuration-driven "Favorite Flower" field) can just use some other Field to actually provide their implementation (like a "standard" field which knows how to render text areas). The previous approach would have involed subclasssing the "standard" field and implementing an interface, but that would mean that every application would have at least two "base" fields and generally just seemed bleh as I worked through it.
The cost of this approach is that we need a bunch of `proxy` junk in the base class, but that's a one-time cost and I think it simplifies all the implementations and makes them a lot less magical (e.g., all of the custom fields now extend the right base field classes).
**Fixed Some Bugs**: Some of this code hadn't really been run yet and had minor bugs.
Test Plan:
{F54240}
{F54241}
{F54242}
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T1702, T1703, T3718
Differential Revision: https://secure.phabricator.com/D6749
Summary:
Fixes T3698. Sometimes views need to render differently depending on whether they contain content or not. The existing approach for this is `isEmptyContent()`, which doesn't work well and is sort of hacky (it implies double-rendering content, which is not always free or side-effect free).
Instead, provide a test for an element without children. This test is powerful enough to catch the easy cases of `null`, etc., and just do the expected thing, but will not catch a View which is reduced upon rendering. Since this is rare and we have no actual need for it today, just accept that as a limitation.
Test Plan:
Viewed Timeline and Feed UI examples. Viewed Feed (feed), Pholio (timelineview), and Differential (old transactionview).
{F53915}
Reviewers: chad, btrahan
Reviewed By: chad
CC: aran
Maniphest Tasks: T3698
Differential Revision: https://secure.phabricator.com/D6718
Summary: Fixes T3641. Probably needs some @chad love though on colors and what have you. Technique was to jam this into the existing notifications stuff as much as possible. I think its "okay" but if we were to add more stuff here (like a 3rd application) this could get a quality pass to consolidate even more code.
Test Plan: played with it in Chrome and Safari - looks reasonable
Reviewers: chad, epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T3641
Differential Revision: https://secure.phabricator.com/D6708
Summary:
Ref T3684. The URI itself is reflected in a few places. It is generally not dangerous because we only let you add random stuff to the end of it for one or two controllers (e.g., the file download controller lets you add "/whatever.jpg"), but:
- Remove it entirely in the main request, since it serves no purpose.
- Remove query parameters in Ajax requests. These are available in DarkConsole proper.
Also mask a few things in the "Request" tab; I've never used these fields when debugging or during support, and they leak quasi-sensitive information that could get screenshotted or over-the-shoulder'd.
I didn't mitgate `__metablock__` because I think the threat is so close to 0 that it's not worthwhile.
Test Plan: Used Darkconsole, examined Requests tab.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T3684
Differential Revision: https://secure.phabricator.com/D6699
Summary: Tightens up the CSS to display more items (4 wide on 15") and fixes some mobile CSS issues with appseach. Fixes T3614
Test Plan: Tested Pholio, Macros, mobile layouts
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T3614
Differential Revision: https://secure.phabricator.com/D6694
Summary: This adds hovercards to most stories and removes the profile photo from one line stories. I don't know about my implementation, which has difficulties with application transactions (because it shows status). Which leads me to a bigger question, which is can we render all people through a common function like AphrontTagView so we can easily class and/or hovercard it anywhere.
Test Plan: Reviewed my feed, various stories.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D6684
Summary: Ref T3671. Depends on D6674. Continues work in D6673, D6674 and extends it into Legalpad and Phriction. Then deletes a bunch of dead code.
Test Plan: Edited documents in Legalpad and Phriction, verified I got reasonable looking previews.
Reviewers: btrahan, Firehed
Reviewed By: btrahan
CC: aran, chad
Maniphest Tasks: T3671
Differential Revision: https://secure.phabricator.com/D6675
Summary:
Ref T3671. A lot of applications have pretty ad-hoc preview code. Clean it up a bit and add Summary preview to Differential.
After ApplicationTransactions we might want to try to serialize the whole form and show a preview of all the transactions, but this seems not very useful in most cases (I'd guess that Remarkup previews are 99% of the value) and tricky to get right (e.g., adding images which don't exist yet to Pholio mocks).
I think I can add this in a few other places, too.
Test Plan:
Edited Maniphest Tasks and Differential Revisions, mashed some buttons. Verified previews rendered correctly. Grepped for removed CSS classes (no hits).
{F52907}
Reviewers: btrahan, Firehed
Reviewed By: btrahan
CC: aran, chad
Maniphest Tasks: T3671
Differential Revision: https://secure.phabricator.com/D6673
Summary: This puts back the 'one line' story we previously had with the updated design.
Test Plan: Review my feed.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D6666
Summary:
Add the ability to select singular and multiple lines in paste to highlight.
This is related to T3627
Test Plan: Create a paste, select one or more lines.
Reviewers: epriestley, tberman
Reviewed By: epriestley
CC: aran, chad
Maniphest Tasks: T3627
Differential Revision: https://secure.phabricator.com/D6668
Summary: It turns out not everything is interesting. This adds a oneline story with less vertical space.
Test Plan: UIExamples
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D6640
Summary:
Ref T3373. The submit listener doesn't properly scope the form it listens to right now, so several forms on the page mean that comments post to one of them more or less at random.
Scope it properly by telling it which object PHID it is associated with.
Test Plan: Made Question comments, saw comments Ajax in on the question itself rather than on an arbitrary answer.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T3373
Differential Revision: https://secure.phabricator.com/D6611
Summary: For Diffusion's auditors list, and Differential's reviewers list (eventually). An inline option might make more sense in Differential, but I want to use this in Diffusion first, and we need more information there.
Test Plan: {F51424}
Reviewers: chad, btrahan
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D6559
Summary: Ref T2715. Switch mocks to the new stuff.
Test Plan: Used `phid.query` and `phid.lookup` to find mocks.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2715
Differential Revision: https://secure.phabricator.com/D6517
Summary:
Ref T3572. Needs some CSS tweaks, but this lets you drag an image on top of another image to replace it. There's no server-side or transaction support (and I'm not planning to build that), I just wanted to clear the way on the JS side.
You'll get an additional array posted called `replaces`. Keys are old file PHIDs; values are new file PHIDs.
Note that a key may not exist yet (if a user adds an image, and then also replaces that same image). In this case, the server should just treat it as an add.
Test Plan: Dragged images on top of other images.
Reviewers: btrahan, chad
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T3572
Differential Revision: https://secure.phabricator.com/D6499
Summary: The feed handler in the IRC bot calls Conduit to render feed stories, which sometimes end up here with no user. "Fix" it. This patch is really bad but the underlying code is a mess too. Fixing this is properly is blocked on T2222/T2217. Until those land, stop fataling at least.
Test Plan: @dctrwatson confirmed this fixed things.
Reviewers: dctrwatson, btrahan
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D6497
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: Ref T3544. For starters, we now need to phutil_implode_html
Test Plan: no more fatals
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T3544
Differential Revision: https://secure.phabricator.com/D6475
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: Ref T2231. Ref T2232. This form doesn't do anything yet and there are no link sto it, but it lets you enter all the data to create a repository in a relatively simple, straightforward way.
Test Plan:
{F49740}
{F49741}
{F49742}
{F49743}
Reviewers: chad, btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2231, T2232
Differential Revision: https://secure.phabricator.com/D6430
Summary:
Ref T2852. Asana has one bug which I'm having a little trouble figuring out. I want to get more information to debug it, but I'll need them to run `bin/feed republish <story_id>` to get that data.
Right now, it's incredibly hard to figure out the story ID for feed stories. So mostly this is to make that easier (click permalink; pull it out of the URL), but it also adds a little functionality and cleans the code up a bit.
The page itself could be prettier and maybe some day we'll add comments or whatever, but it seems reasonably functionalish.
Test Plan:
{F49962}
- Also loaded many pages of feed history to check that nothing broke.
Reviewers: btrahan, chad
Reviewed By: chad
CC: chad, aran
Maniphest Tasks: T2852
Differential Revision: https://secure.phabricator.com/D6440
Summary:
Fixes T2691. Now, all PhabricatorActionListViews in the codebase setObjectHref to $request->getRequestURI. This value is passed over to PhabricatorActionItems right before they are rendered. If a PhabricatorActionItem is a workflow and there is no user OR the user is logged out, we used this objectURI to construct a log in URI.
Potentially added some undesirable behavior to aggressively setUser (and later setObjectURI) from within the List on Actions... This should be okay-ish unless there was a vision of actions having different user objects associated with them. I think this is a safe assumption.
Test Plan: played around with a mock all logged out (Ref T2652) and it worked!
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2691
Differential Revision: https://secure.phabricator.com/D6416
Summary: Fixes T3525. This feels way better, although it's still a little hard for me to pick out of lists with otherwise default-colored items.
Test Plan: {F49910} {F49911}
Reviewers: chad
Reviewed By: chad
CC: aran
Maniphest Tasks: T3525
Differential Revision: https://secure.phabricator.com/D6435
Summary: While the icons are sometimes pretty obvious ("X" for delete, "pencil" for edit), some are more obscure ("Disable", "Ignore"), especially if you haven't seen the interface before. If a name is available, render it as a tooltip.
Test Plan: {F49905}
Reviewers: chad, btrahan
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D6434
Summary: See discussion in D6403.
Test Plan: {F49488}
Reviewers: chad, btrahan
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D6409
Summary:
This leaves the space between the properties and the blurb looking a bit empty, but there will be more stuff there soon (status, VCS names, email, phone/fax numbers, etc., and custom user fields).
I removed "view lint messages" since I'm pretty sure no one has ever clicked it. I think providing better search (e.g, T2625) to that UI in Diffusion is a preferable approach.
Test Plan: {F49423}
Reviewers: chad, btrahan
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D6403
Summary:
Ref T1703. Drive "user since" with a custom field and make the other fields render into a property list.
Users can make their profiles a little more personal/obnoxious now.
Also delete a bunch of code.
Test Plan: {F49415}
Reviewers: chad, btrahan
Reviewed By: chad
CC: aran
Maniphest Tasks: T1703
Differential Revision: https://secure.phabricator.com/D6401
Summary:
We have this old view which is only used in two places and looks the same but has totally different markup. Get rid of it.
@chad, I'm generally going to move the user/project profiles a step toward looking like other object detail view with the custom field stuff. Not sure if you have any grand vision here; we can easily do something else later since this is like 80% "delete weird epriestley one-offs that don't look quite right in favor of standard elements".
Test Plan: {F49324} {F49325} {F49326}
Reviewers: chad, btrahan
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D6394
Summary: This reverts commit e70bb28ea0. We didn't end up using these.
Test Plan: Looked at Differential.
Reviewers: chad
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D6357
Summary: Ref T3485. Moves flag icon inline in the header.
Test Plan: {F48654}
Reviewers: chad
Reviewed By: chad
CC: aran
Maniphest Tasks: T3485
Differential Revision: https://secure.phabricator.com/D6355
Summary:
Ref T603. Ref T2625. Fixes T3241. Depends on D5451. Depends on D6346.
@wez, this changes the Differential revision list UI substantially and may generate a lot of bikeshedding / who-moved-my-cheese churn. See T3417 for context, for example. The motivations for this change are:
- The list now works on devices, like phones and tablets. This is a requirement to make the rest of Differential work on devices.
- Although ApplicationSearch intentionally presents a simpler interface initially and some options which were one click away before aren't now, it is much more powerful than the search it replaces and allows users to build, save, share, fork, edit, and customize a much wider range of queries. Users who used the old filters frequently can use Advanced Search -> Save Custom Query to create new versions of them, and of any other query. "Edit Queries.." allows users to remove and reorder queries, including builtin queries. Basically, there are like three things which have gone from "1-click" to "a few clicks", and ten trillion things which have gone from "hard/impossible" to "relatively easy".
The local screenshots look a bit iffy, but I think a lot of this is the fakenesss of my test data. If they still feel iffy in production we can tweak them until they feel good, like we did for Maniphest.
Test Plan:
{F48477}
{F48478}
Reviewers: btrahan, chad, wez
Reviewed By: btrahan
CC: aran, s
Maniphest Tasks: T603, T2625, T3241
Differential Revision: https://secure.phabricator.com/D6347
Summary: They seem to look OK?
Test Plan: {F48529} {F48530}
Reviewers: chad
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D6350
Summary:
Addes a button group to filter tasks by agents, non-agents or all.
Fixes T3394
Test Plan: View task list, filter by agents, filter by non agents. Make sure the correct tasks display.
Reviewers: epriestley, dctrwatson
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T3394
Differential Revision: https://secure.phabricator.com/D6328
Summary:
Ref T2852. Primarily, this expands API access to Asana. As a user-visible effect, it links Asana tasks in Remarkup.
When a user enters an Asana URI, we register an onload behavior to make an Ajax call for the lookup. This respects privacy imposed by the API without creating a significant performance impact.
Test Plan: {F47183}
Reviewers: btrahan
Reviewed By: btrahan
CC: chad, aran
Maniphest Tasks: T2852
Differential Revision: https://secure.phabricator.com/D6274
Summary:
Ref T1536.
- When users try to add a one-of provider which already exists, give them a better error (a dialog explaining what's up with reasonable choices).
- Disable such providers and label why they're disabled on the "new provider" screen.
Test Plan:
{F47012}
{F47013}
Reviewers: chad, btrahan
Reviewed By: chad
CC: aran
Maniphest Tasks: T1536
Differential Revision: https://secure.phabricator.com/D6256
Summary: Added in color variables in most used places. Tweaked green to be a bit more serious.
Test Plan: Tested Tags, Error View, Timeline, Object Views, and Color Palette.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D6244
Summary: Picked a set of standard colors. Based on our current Maniphest color set, but tweaked to the same hue with http://color.hailpixel.com/
Test Plan: Not intended to be end all be all, but a decent first cut. Applied to Maniphest and Tags.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D6229
Summary:
Ref T1536.
- When we render a dialog on a page by itself, put it on a dust background.
- Currently, we render "Logout" in two different places. Stop doing that.
- Make sure the surviving one has workflow so we get a modal ajax dialog if possible.
Test Plan: {F46731}
Reviewers: chad, btrahan
Reviewed By: chad
CC: aran
Maniphest Tasks: T1536
Differential Revision: https://secure.phabricator.com/D6226
Summary: Touches a lot of little spacing things here and there, stuck to 4px grid when possible, checked mobile views.
Test Plan: Mobile, Logging In, Multiple Providers.
Reviewers: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D6220
Summary:
Ref T1536. Currently, we have separate panels for each link/unlink and separate controllers for OAuth vs LDAP.
Instead, provide a single "External Accounts" panel which shows all linked accounts and allows you to link/unlink more easily.
Move link/unlink over to a full externalaccount-based workflow.
Test Plan:
- Linked and unlinked OAuth accounts.
- Linked and unlinked LDAP accounts.
- Registered new accounts.
- Exercised most/all of the error cases.
Reviewers: btrahan, chad
Reviewed By: btrahan
CC: aran, mbishopim3
Maniphest Tasks: T1536
Differential Revision: https://secure.phabricator.com/D6189
Summary: Ref T1536. Error state is a bit gross but we need to sort that out in general.
Test Plan:
{F46549}
{F46550}
Reviewers: chad, btrahan
Reviewed By: chad
CC: aran
Maniphest Tasks: T1536
Differential Revision: https://secure.phabricator.com/D6208
Summary: Rough pass at a PHUIButtonView Class. Keeps phutil_tag intact and adds some image features if you use the class.
Test Plan: UIExamples
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D6192
Summary: Fixes T3330
Test Plan: Test desktop and mobile menus in chrome and ios.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T3330
Differential Revision: https://secure.phabricator.com/D6157
Summary: Adds collapsing of the sidebar, also allows you to say where it goes on mobile (above or below content). ToC for example, above. General Navbar, below. Up to you.
Test Plan: Review UIExamples and Diviner.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D6150
Summary: Gets TOC populated for articles, at least, and fixes a few other things.
Test Plan: {F45474}
Reviewers: chad
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D6144
Summary: See discussion in D6131, D6130. This turned into 35 layers of mess so throw it away and just tweak the JS to be more flexible.
Test Plan:
- Clicked "Show More Applications".
- Clicked "Show Fewer Applications".
- Edited tasks using popup dialog.
- Tried to drag tasks using pencil icon (correctly no longer works).
- Changed threads in Conpherence.
- Not sure how to actually hit the Conpherence "Load ... Threads" thing since
it seems to auto-load? But that works, at least, and the code doesn't really
care what you hit.
- Added a conpherence participant.
- Added a new calendar item.
- Poked around other menus.
Reviewers: chad
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D6133
Summary: This adds examples and abstracts out CSS for common nav re-use.
Test Plan: Tested DocumentExample and ListExample
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D6138
Summary:
This diff covers a bit of ground.
- PHUIDocumentExample has been added
- PHUIDocument has been extended with new features
- PhabricatorMenuView is now PHUIListView
- PhabricatorMenuItemView is now PHUIItemListView
Overall - I think I've gotten all the edges covered here. There is some derpi-ness that we can talk about, comments in the code. Responsive design is missing from the new features on PHUIDocument, will follow up later.
Test Plan: Tested mobile and desktop menus, old phriction layout, new document views, new lists, and object lists.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D6130
Summary: See Conpherence discussion. This probably works?
Test Plan:
Didn't test at all!
bwahaha
Reviewers: chad
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D6120
Summary: Provide all the setSigil() / setID() sorts of calls. No functional changes.
Test Plan: Viewed object item lists.
Reviewers: chad, btrahan
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D6112
Summary:
Ref T988. Fixes T3150. I want to use this element in Diviner, so separate it from Phriction.
This makes no changes to the actual display except for fixing {T3150} by adding `overflow: hidden;`.
Test Plan: Viewed Phriction documents in mobile and desktop views.
Reviewers: chad, btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T988, T3150
Differential Revision: https://secure.phabricator.com/D6101
Summary: Ref T2625. Ref T1163. A couple of small generalization nudges, but this is almost entirely straightforward.
Test Plan: Executed various File queries.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T1163, T2625
Differential Revision: https://secure.phabricator.com/D6091
Summary:
Ref T1163. Ref T2625. This could probably use some tweaks, but I kept things mostly-generic.
I added a new control for freeform dates so we can have it render help or whatever later on.
Test Plan: See screenshots.
Reviewers: chad, btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2625, T1163
Differential Revision: https://secure.phabricator.com/D6084
Summary:
Fixes T3279. For ApplicationSearch (and in some other cases) I'd like users to be able to provide an optional date. This isn't currently possible.
Add a checkbox which disables or enables the input.
Test Plan: Used UIExample to enter dates. Used Calendar to enter dates.
Reviewers: chad, btrahan
Reviewed By: chad
CC: aran
Maniphest Tasks: T3279
Differential Revision: https://secure.phabricator.com/D6082
Summary:
Fixes T3252. Other enhancements:
- Header in widget panel was 2px too short.
- Typeahead in add people only allowed one person
- Typeahead in add people was cutoff by overflow:hidden
- X in remove has been changed to unicode (multiply)
- Add people dialog form fields are full width
- Some other CSS tweaks.
Test Plan: Add, Remove people.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T3252
Differential Revision: https://secure.phabricator.com/D6076
Summary:
Ref T2625. @chad, you might have some feedback here. The behaviors this implements are:
- When the user selects "Advanced Search", we show the full search UI and no results (for performance and clarity).
- When the user submits a search which //is not// a named search, we show the full search UI and the "Save Custom Query..." button.
- When the user submits a search which //is// a named search, we show "Results for search X." with an "Edit Query..." button. The button expands the search form.
- When the user selects a builtin query (like "All Pastes"), we don't show any search UI, but I'm probably going to make this behave more like named searches.
Test Plan:
{F44346}
{F44347}
Reviewers: chad, btrahan, blc
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2625
Differential Revision: https://secure.phabricator.com/D6063
Summary: Add a button to the Remarkup area that explains how to attach an image and remove the separate upload field.
Test Plan: Check that the dialog pops up correctly and that dropping images onto the Remarkup area works.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin, AnhNhan
Maniphest Tasks: T879
Differential Revision: https://secure.phabricator.com/D6049
Summary:
Kind of just guessed what to do here. It seemed reasonable.
Take the transaction, use the author to get an actor image. Get the main object handle type, use that for the app icon. Use the transaction's created date as a time.
Ref T2988
@chad I think there's still no icon for mock?
Test Plan: Look at my feed. See some info.
Reviewers: epriestley, chad
Reviewed By: epriestley
CC: chad, aran, epriestley, Korvin
Maniphest Tasks: T2988
Differential Revision: https://secure.phabricator.com/D6044
Summary: Ref T3155. Also re-adds the ability to update Conpherence titles by letting user click the title and fill out a little dialogue. Also fixes a bunch of random bugs and what have you. I tried to make the javascript less mysterious by trying to code what's actually happening more explicitly. Still a work in progress all over the place but a good stopping point for feedback.
Test Plan: played around with Conpherence. In particular, went to /conpherence/ and re-sized and went to /conpherence/X/ and re-sized. Also loaded up my no conpherneces user.
Reviewers: epriestley
Reviewed By: epriestley
CC: chad, aran, Korvin
Maniphest Tasks: T3155
Differential Revision: https://secure.phabricator.com/D6022
Summary:
Ref T2319. In a property list view, if there's a property like:
Property Name propertyvalue
...and you double click "propertyvalue" to select it so you can copy paste it (maybe it's a branch name or a URI), you get "Namepropertyvalue" (or worse). Although this "fix" is hacky, it seems to work alright and not cause side effects.
Adding spaces between the tags instead (`<dt>x</dt> <dd>y</dd>`) did not work in Safari. Adding a `<span> </span>` between the tags did, but that seems even messier.
Test Plan: Double-clicked "propertyvalue".
Reviewers: chad, btrahan
Reviewed By: chad
CC: aran
Maniphest Tasks: T2319
Differential Revision: https://secure.phabricator.com/D6026
Summary:
Ref T2232. Very busy day on IRC so I feel like I've made 20 minutes of progress in 1-minute spurts here, but this adds the basics for a form that can have multiple pages and automatically handle pagination and reading to/from the request, objects and responses.
The UIExample is reasonably instructive. Basically, you make a form, add pages to the form, and add controls to the pages. The core flow control looks like this:
if ($request->isFormPost()) {
$form->readFromRequest($request); // (1)
if ($form->isComplete()) { // (2)
$response = $form->writeToResponse($response); // (3)
// Process result here. // (4)
}
} else {
$form->readFromObject($object); // (5)
}
The key parts are:
# This reads the form state from the request, including reading all the inactive pages.
# This tests if all pages are valid and the user just clicked "Done" on the last page.
# This produces a "response", which might be writing to an object (for simpler forms) or creating a transaction record (for more complex forms).
# Here, we would save the object or apply the transactions.
# When the user views the form for the first time, we preload all the values from some object (which might just be empty).
Ultimate goal here is to fix repository creation to not be a terrible pit of awfulness.
There are probably a lot of rough edges and missing features still, but this seems to not be totally crazy.
I'm using two submit buttons with different names which doesn't work on IE7 or something, but we can JS our way out of that if we need to.
Test Plan: Paged forward and backward through the form.
Reviewers: btrahan, chad
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2232
Differential Revision: https://secure.phabricator.com/D6003
Summary: This adds the ability to have a multi-column full height container that is responsive based on PHUIBox's shadow box.
Test Plan: Tested new examples in UIExamples and Workboards.
Reviewers: epriestley, edward, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5996
Summary: This piggybacks onto device-phone's CSS rules to enable a full width form (for smaller spaces).
Test Plan: Convert New Message dialog to fullWidth.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5924
Summary: Some places we pass in empty strings and get hanging middots. Would prefer not have the middot, and let the developer decide if they should set a 'nothing here' state.
Test Plan: Checked Repositories with no lint messages.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5920
Summary: `getAvailableIcons` parses icon(s).json, which has recently been pluralized in D5890.
Test Plan: /uiexample/view/PhabricatorActionListExample/
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5902
Summary:
We have a few interfaces where add "Edit", "Delete" or some other action to a list. Currently, this happens via icons, but these are cumbersome and weird, are inconsistent, can't be workflow'd, are hard to hit on desktops and virtually impossible to hit on mobile, and generally just feel iffy to me. Prominent examples are Projects and Flags. I'd like to try adding an "edit" action to Maniphest (to provide quick edit from list views, basically). It looks like some of Releeph would benefit here, as well.
Instead, provide first-class actions:
{F42978}
They produce targets which my meaty ham-fists can plausibly hit on mobile, too:
{F42979}
(We could do some kind of swipe-to-expose thing eventually, but I think putting them by default is OK?)
Test Plan: Added UIExamples. Checked desktop/mobile.
Reviewers: chad, btrahan, edward
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D5890
Summary: 1) Borders were appearing on inputs not as expected. 2) the SWF container was always displayed at the bottom of every page load (long time issue). There are more issues, but this fixes the 2 largest for right now.
Test Plan: Tested Maniphest create page.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5867
Summary:
This creates a common form look and feel across the site. I spent a bit of time working out a number of kinks in our various renderings. Some things:
- Font Styles are correctly applied for form elements now.
- Everything lines up!
- Selects are larger, easier to read, interact.
- Inputs have been squared.
- Consistant CSS applied glow (try it!)
- Improved Mobile Responsiveness
- CSS applied to all form elements, not just Aphront
- Many other minor tweaks.
I tried to hit as many high profile forms as possible in an effort to increase consistency. Stopped for now and will follow up after this lands. I know Evan is not a super fan of the glow, but after working with it for a week, it's way cleaner and responsive than the OS controls. Give it a try.
Test Plan: Tested many applications, forms, mobile and tablet.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5860
Summary: There are a few places a third text row in ObjectItemListView is needed or make things easier to read. Built and rolled out in Config.
Test Plan: Tested in Config Groups.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5837
Summary: gotta idx() on this as the user may have no conpherences at all.
Test Plan: loaded up my 'noconpherences' user and observed no errors in the error log. my normal test user still worked okay too.
Reviewers: epriestley, garoevans
Reviewed By: garoevans
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5791
Summary: this is D5750 but just the conpherence part. fixes a few random conpherence bugs / quirks as well. Also messes with ApplicationTransactionEditor to expose the xactions so Conpherence doesn't over-update participation rows. Fixes T2429.
Test Plan: set LIMIT to 3. verified I could scroll down all conpherences. next, picked a conpherence "in the middle" to load. verified I could page up and down. next, picked a conpherence in the middle then had another user update that conpherence. verified as I paged up the conpherence re-loaded properly selected
Reviewers: epriestley
Reviewed By: epriestley
CC: chad, aran, Korvin, vrana
Maniphest Tasks: T2429
Differential Revision: https://secure.phabricator.com/D5783
Summary: This provides some new display methods and examples to PHUIFeedStory.
Test Plan: Tested UIExamples Page, mobile layouts, and existing Feed Pages (feed, profile, etc). I want to add a bit more but am stopping here since it's not a priority.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5749
Summary:
Ref T2599.
Implements an "order" mode which fullscreens the editor and reduces distractions, similar to Asana's "focus" mode and GitHub's "zen" mode. This can help users who need fewer distractions get work done.
Implements a "chaos" mode which does the opposite. This can help users who need more distractions to get work done.
Test Plan: Clicked "order" and "chaos" buttons.
Reviewers: chad, btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2599
Differential Revision: https://secure.phabricator.com/D5735
Summary: Adds a base class for displaying images and icons.
Test Plan: Tested giving and taking tokens, viewed action headers, uiexamples for icons, workboards.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5736
Summary:
I feel that remarkup code blocks are still kind of small. This should help with my reading problems. Hesitating to add `.PhabricatorMonospaced` to the remarkup rule, since it's pretty specific to Phabricator.
Pasting in the diff btw, since my `arc`'s going crazy (doubt that's `arc.exe`, since it's not landed yet).
Test Plan:
Set font to something large. viewed code block. Was not large.
Applied change. Viewed code block again. It was //**HUGE**//!
Also verified that the `init.js` was still there. I think that's the one being inserted `$5`.
Reviewers: epriestley, chad
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5727
Summary: Adds a basic div box that takes some styles. Not sure this is the best approach for the spacing, but overall hoping people can spend less time in CSS and just use this class.
Test Plan: UIExamples
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin, AnhNhan
Differential Revision: https://secure.phabricator.com/D5723
Summary: Does a few things, standardizes feed a bit more on people, projects. Cleans up Project pages to be more dashboard like. Adds usable mobile support. Remove extenal public feed styles. The Project pages won't win any design awards, but they are much more usable and responsive (mobile). I assume the default view to be workboard still at some point.
Test Plan: Test out Profile, Project Profile, Public Feed, normal Feed. Mobile and Chrome.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5700
Summary: This moves Feed's rendering over to a PHUI class. I want to build it out and have it power Ponder, Phame, Feed, as well as Profiles and Projects in some fashion. It also provides some more data depth over ObjectItemView. Also updated Profile for mobile and fixed some other display issues there.
Test Plan: Tested Feed, Profile. Used iOS and Chrome.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin, AnhNhan
Differential Revision: https://secure.phabricator.com/D5687
Summary: Refreshes feed's design a bit, adds app icons, works nicer on mobile.
Test Plan: Tested many feed stories, not sure I got them all, but seems fine.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5673
Summary: This is mostly minor, but visually it makes the wiki feel more 'page like' and better separates the actual content from other data displayed.
Test Plan: Tested Chrome, iPhone, and iPad.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin, AnhNhan
Maniphest Tasks: T2686
Differential Revision: https://secure.phabricator.com/D5366
Summary: Adds in the ActionList into Crumbs for mobile on many applications.
Test Plan: Tested each application except probably drydock since not sure how to test that. Also cleaned up Ponder a little.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5648
Summary: We're baking some useful things into ActionHeader, would like to consolidate it's use around the site for consistency.
Test Plan: Tested log out dialog, attach dependencies, delete document in phriction.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin, AnhNhan
Differential Revision: https://secure.phabricator.com/D5635
Summary: First pass here, still needs some UI work (maybe? tablets?). Getting it out to see if implementation is corrrect.
Test Plan: Shrink Maniphest task on Chrome, see new UI. Toggle menu. Test on iOS.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5631
Summary: Tightens up spacing, remove some of the borders, add alpha channel, make them all blue (sorry, red green and yellow are for 'status'). If we want to do more colors just for hovercards, I have a brown and a black in the mock, but would like to try just blue for now.
Test Plan: UIExamples, Tasks, People, Diffs, and Pastes.
Reviewers: epriestley, AnhNhan, btrahan
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5609
Summary:
Also join concepts of installed and enabled applications.
Also respect uninstalled Maniphest where disabled Maniphest was checked.
Test Plan:
Visited T1, D1.
Uninstalled Maniphest then visited T1, D1.
Disabled Maniphest then visited T1.
Visited /config/edit/maniphest.enabled/.
Reviewers: epriestley, Afaque_Hussain, edward
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5602
Summary: At least for non-workboard views, try plain text for author information instead of profile images. Some discussion in D5451.
Test Plan:
{F39411}
{F39412}
{F39413}
{F39414}
{F39415}
{F39416}
Reviewers: chad
Reviewed By: chad
CC: AnhNhan, aran
Differential Revision: https://secure.phabricator.com/D5605
Summary:
Currently, `isInstalled()` and `getAllInstalledApplications()` are inconsistent:
- `isInstalled()` returns true for beta apps, even if `phabricator.show-beta-applications` is false.
- `getAllInstalledApplications()` omits beta apps if `phabricator.show-beta-applications` is false.
Making the beta config control installs (not just homepage visibility) makes far more sense as we roll out more thorough application integrations.
Make `isInstalled()` respect beta, and clean up some callsites.
D5602 builds on this.
Test Plan: Installed/uninstalled beta apps, verified Conpherence menu/panel and other application integrations dropped out of the UI.
Reviewers: vrana, btrahan
Reviewed By: vrana
CC: aran
Differential Revision: https://secure.phabricator.com/D5603
Summary:
Refs T1048; Depends on D5557, D5558
They hover right above your eyes. Try it out at home :D (in that case, don't forget to checkout D5557 and D5558)
Worth a lot of improvement. But it's great for now after a little bit of styling.
Scrape load (search current document for all hovercards and pre-load them) and lazy load (e.g. previewing comments which is not covered by scrape load) implemented.
Added some seemingly useful graceful situations. Too much to the left, too much to the top.
Test Plan:
Tested on Ubuntu, Chrome and FF. No Windows yet, since I have it live at no place. Works pretty fine, at least it will appear.
Could test left graceful fallback. Don't know what happens if you place it too far to the top. I expect either good results or placement about the center of the screen in case of glitches.
For now, they'll disappear right away once the mouse leaves the object tag. Intended is when leaving both tag and hovercard.
Reviewers: epriestley, chad, btrahan
CC: aran, Korvin
Maniphest Tasks: T1048
Differential Revision: https://secure.phabricator.com/D5563
Conflicts:
src/__celerity_resource_map__.php
Summary:
Refs T1048; Depends on D5571 - Use names instead of `substr($handle->getType(), 0, 1).$handle->getAlternateID()`, which did not work for 70% of the handles
This, of course, breaks UI Examples, which was pretty off before anyway.
Unbreak UIExamples.
Test Plan: UIExamples, Hovercard Controller in human test mode
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T1048
Differential Revision: https://secure.phabricator.com/D5572
Summary: Initial pass at an action header. The idea is to support current and future planned needs in 'headers' with various colors and icons. The overall goal here is to keep markup light and allow other classes to wrap and extend with more specific features.
Test Plan: Tested UIExamples and Workboards.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin, AnhNhan
Differential Revision: https://secure.phabricator.com/D5585
Summary: Refs T1048 - Adds sigil and metadata to emitted tags.
Test Plan: Inspected HTML, verified sigil and meta data reference. Hooked up behaviours in a future broken diff. Could successfully load hovercards from endpoint. I'm bad with JS though.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T1048
Differential Revision: https://secure.phabricator.com/D5558
Summary: Removes the glow icons and uses a hover change. Fixes phantom anchors.
Test Plan: Review in Chrome at various sizes (phone tablet). Check that icons still work. Check that mobile menus render when clicked.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2876
Differential Revision: https://secure.phabricator.com/D5549
Summary:
Refs T1048
- Add IDs to hovercard view
- Fixed title
- Added fatal for no handle
Test Plan: Used in future diff. Verified fatal for no handle. Correct display of title and id
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T1048
Differential Revision: https://secure.phabricator.com/D5543
Summary: Abstract out the multi-column code from workboards and have it be available separately. I feel like there will be some benefit here especially for custom developers in how they present infromation (like releeph). It also scales back to tablet and mobile fairly well, so they get those things for free.
Test Plan: Tested mobile, tablet and chrome layouts.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5529
Summary:
Refs T1048 - I'm pretty happy and happy to tell you
Savepoint CR
Everything's dummy right now. If you are testing locally, don't forget to edit the PHIDs. Or populate your own handles. Doesn't really matter.
I'm mainly sending it in for the CSS, not the messy PHP code. Ignore the `margin: auto`, that's just for looking nice.
Test Plan: UI Example » Hovercard
Reviewers: epriestley, chad
CC: aran, Korvin
Maniphest Tasks: T1048
Differential Revision: https://secure.phabricator.com/D5519
Summary: Exactly what it says on the tin
Test Plan: No shadow when visiting saved custom queries in Maniphest. Still appears normally for other use sites
Reviewers: epriestley, chad
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5535
Summary: Use UA strings to detect platform; override general monospaced settings with platform-specific ones. Fixes T2868.
Test Plan: whatcouldgowrong
Reviewers: chad
Reviewed By: chad
CC: aran
Maniphest Tasks: T2868
Differential Revision: https://secure.phabricator.com/D5526
Summary: Adds an action panel on the left side of the workboard.
Test Plan: Tested Fluid and mobile layouts
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5496
Summary: Adds action items in the footer of workpanels.
Test Plan: UIExamples on Chrome.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5492
Test Plan: Double clicked on the last word of title. Tag wasn't selected.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5462
Summary: Adds Workboards and workpanels. This is a preliminary diff, I'm still working on mobile and tablet and a few missing features (header actions)
Test Plan: FF, Chrome, iOS, iPad, iPhone, IE
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5455
Summary: Adds ability to add 20, 10 and 5 px of margin or padding without adding additional rules.
Test Plan: Reloaded any Phabricator Page. It didn't break.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5477
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 an icon if you add Phabricator as a web app on your iPhone.
Test Plan: Test on simulator and in iOS.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5444
Summary:
Ref T2826.
- Adds a min height (arbitrarily, height of the gradient; other choices are 60px [title + avatar], or 70px [title + projects + closed + avatar]).
- Color bars 4px -> 6px.
- Fixes profile image clipping in Firefox, etc.
- Removes background color from avatars, for transparent GIFs and such.
- Fixes shift-click to select tasks in views that can't be grabbed.
Test Plan: {F37535}
Reviewers: chad
Reviewed By: chad
CC: aran
Maniphest Tasks: T2826
Differential Revision: https://secure.phabricator.com/D5436
Summary:
This isn't quite complete, but everything else is technical cleanup. Broadly:
- Removed checkboxes. Selected state is now indicated with CSS, and toggled with shift-click. When nothing is selected, the text reads "Shift-Click Tasks to Select" to let users discover this feature.
- Updated drag-to-reorder code to work with ObjectItemListView.
- Closed/resolved is now shown with a grey footer icon.
- Assigned is now shown with a user profile image handle icon, with a hover state.
This could probably use some more tweaks, but overall I think it looks pretty reasonable?
Test Plan: {F35897}
Reviewers: chad
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D5340
Summary:
Safari has a weird bug with `border-radius` plus border color:
{F35865}
Move the uncolored borders to an internal div to fix this. Also tweak some positioning on icons for cards, and add a "magenta" color.
Test Plan: {F35866}
Reviewers: chad, btrahan
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D5338
Summary: Mostly finished, wanted to get it into your hands to play with. I need to remove some more dead CSS and figure out where we want to put profile/logout, but overall feels pretty good. Tested a bunch in iOS and other layouts.
Test Plan: Test Home, Maniphest, search bars, app menus.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin, AnhNhan
Maniphest Tasks: T2809
Differential Revision: https://secure.phabricator.com/D5407
Test Plan: removed one or two labels from uiexample, verified expected behaviour
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5382
Summary:
does the title and also a few other small tweaks
- kills the init js behavior; now its part of menu where it belongs.
- adds the underline to the icon when its toggled in the widget menu
- fixed JS initialization errors on the "create conpherence" page. Note I still like keeping all that init stuff in one function because its typing the same data a bunch to be passed over to the JS layer. Other ways to accomplish this obvi...
Only fun wrinkle here is I think Chad intended me to display "when the file was attached". Instead, I display when the file was *uploaded*. I think this jives better with our version where you can't delete and all that. Files are pretty powerful, long-living objects in Phabricator land.
Test Plan: added files to a conpherence and noted widget loaded updated okay. added a file with no author (generated by the system) and verified it still rendered okay. switched between conpherences and verified proper data in each files widget. uploaded image and text files to check the icons were correct.
Reviewers: epriestley, chad
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2530
Differential Revision: https://secure.phabricator.com/D5337
Summary:
Probably not the ideal way to deal showing only certain amount of lines. Size vary by browsers,
zooming will mess it up albeit only a little and will definitely not work with IE.
Test Plan: {F35989}
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T1770
Differential Revision: https://secure.phabricator.com/D5347
Summary:
- Added support for highlighting to PhabricatorSourceCodeView
- Added support for highlighting to embed pastes
Test Plan: {F35975}
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T1770
Differential Revision: https://secure.phabricator.com/D5346
Summary: Same as title
Test Plan: By checking in Phriction UI in Phabricator
Reviewers: epriestley, AnhNhan
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5327
Summary:
A few things
- pht Maniphest where I could
- implement dust background
- optimize pages for mobile
- adds aphront-two-column-layout
- reworks maniphest page with two column layout
- tweaks task table for mobile, though we should move to object-list-view
Stopping here as I want to get feedback in. Super excited about mobile and the new tasks views. Only sort of excited about the sidebar filters, they need more UI work, but we should talk about that.
Test Plan: Test Maniphest, Differential, and Homepage views. Sort tasks, make reports
Reviewers: epriestley, btrahan
Reviewed By: btrahan
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5314
Summary: Like AphrontFormToggleButtonsControl, but with mouseover counters. These counters let you know how many of each thing there are in each category, which is useful when using this control for filtering a list of things in multiple dimensions.
Test Plan: `/uiexample/view/PhabricatorCountedToggleButtonsExample/`
Reviewers: epriestley
CC: aran, Korvin
Maniphest Tasks: T2094
Differential Revision: https://secure.phabricator.com/D5118
Summary: Wanted to pull this out in case we don't use it in Maniphest, still useful perhaps in the future. Creates a sidebar that wraps when on mobile.
Test Plan: Tested UIExample
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5321
Summary:
- Grouping inline comments made us render `array(null)` for the content of a single comment. Detect that this corresponds to empty text.
- Don't try to mention-subscribe automatic subscribers, like the author.
Test Plan: Tried to mention-subscribe the author, saw no "x subscribed y" transaction generate. Added a real subscriber, saw no empty content render.
Reviewers: chad
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D5315
Summary:
Fixes T2639 by grouping related transactions at display time, so all the inlines merge into a nice block.
(Note that this does not do anything about T2709 yet, so there's still no way to figure out where the inlines actually are.)
Test Plan: {F35262}
Reviewers: chad
Reviewed By: chad
CC: aran
Maniphest Tasks: T2639
Differential Revision: https://secure.phabricator.com/D5313
Summary:
Initial pass at elements appearing on M10.
Glaring omissions:
- I cut a single icon out of M10 in a haphazard way.
- No linear graident texture on the cards.
Test Plan:
{F35248}
{F35249}
Reviewers: chad
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D5311
Summary:
Ref T2700. Allow JS to listen for swipes on devices.
There are a bunch of tricky cases here and I probably didn't get them all totally right, but this interaction broadly looks like this:
- We implement gesture recognition for the mouse in device modes (narrow browser), and for touch events from an actual device.
- The sigil `touchable` indicates that a node wants to react to touch events.
- When the user touches a `touchable` node, we start listening for moves. They might be tapping/clicking (in which case we don't care), but they might also be gesturing.
- Once the user moves their finger/pointer far enough away from the tap origin, we recognize it as a gesture. I hardcoded this at 20px; I wasn't able to find any "official" Apple value, but 20px seems like a common default.
- At this point, we look at where their finger has moved.
- If they moved it mostly up/down, we interpret the gesture as "scroll" and just stop listening. The device does its own thing.
- However, if they moved it mostly left/right, we interpret it as a "swipe". We start killing the moves so the device doesn't scroll.
- Once we've recognized that a gesture is underway, we send a "gesture.swipe.start" event and then "gesture.swipe.move" events for every move.
- When the user ends the gesture, we send "gesture.swipe.end".
- If the user cancels the gesture (currently, only by tapping with a second finger), we send "gesture.swipe.cancel".
- Gesture events have raw position data and some convenience fields.
Test Plan:
Wrote UI example and used it from the Desktop, iPhone simulator, and a real iphone.
- The code always seems to get "scroll" vs "swipe" correct (i.e., consistent with my intentions).
- The threshold feels pretty good to me.
- Tapping with a second finger cancels the action.
Reviewers: chad, btrahan
Reviewed By: chad
CC: aran
Maniphest Tasks: T2700
Differential Revision: https://secure.phabricator.com/D5308
Summary: After D5305, this method does nothing since we automatically figure out what we need to do.
Test Plan:
- Viewed a page with the main menu on it (MainMenuView).
- Viewed a revision with transactions on it (TransactionView).
- Viewed timeline UIExample (TimelineView, TimelineEventView).
- Viewed a revision (PropertyListView).
- Viewed a profile (ProfileHeaderView).
- Viewed Pholio list (PinboardView, PinboardItemView).
- Viewed Config (ObjectItemView, ObjectItemListView).
- Viewed Home (MenuView).
- Viewed a revision (HeaderView, CrumbsView, ActionListView).
- Viewed a revision with an inline comment (anchorview).
- Viewed a Phriction diff page (AphrontCrumbsView).
- Filed T2721 to get rid of this.
- Looked at Pholio and made inlines and comments (mockimages, pholioinlinecomment/save/edit).
- Looked at conpherences.
- Browsed around.
Reviewers: chad, vrana
Reviewed By: chad
CC: edward, aran
Differential Revision: https://secure.phabricator.com/D5307
Summary: Allows views to work like tags.
Test Plan: Implemented a few completely arbitrary render() / singleView simplifications. I just picked some that were easy to test. I'll do a more thorough pass on this in a followup; these calls don't really hurt anything.
Reviewers: chad, vrana
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D5306
Summary: files widget updates as new files are added. made basically all edits "ajax" except for when you change the conpherence image which just does a reload. I will fix this in a future diff but it was starting to spiral out of control a bit.
Test Plan: commented on a task with files and noted the updated file widget. updated header image via drag and drop and dialogue -- noted a full reload. cropped image and re-titled conpherence - ajax updated as expected.
Reviewers: epriestley, chad
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2530
Differential Revision: https://secure.phabricator.com/D5288
Summary: This is supposed to look a lot like the way Remarkup renders a block of code, so you can render some out of context message inside another container. For example in Releeph, it renders a message someone has associated with a Releeph request.
Test Plan:
I've added an abstract uiexample, but the use case in Releeph is more explanatory:
{F33900}
Reviewers: epriestley, chad
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2094
Differential Revision: https://secure.phabricator.com/D5125
Summary: Aphront widgets that render the either a discrete or continuous value as a horizontal shape. Like a progress bar, or a five-star rating bar.
Test Plan:
`/uiexample/view/PhabricatorAphrontBarExample/` ...which shows this, amongst other things:
{F33898}
Reviewers: epriestley, chad
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2094
Differential Revision: https://secure.phabricator.com/D5122
Summary:
For a single line, I can use the right click.
But for highlighting multiple lines, I have to wait for page reload.
It would be nice to track this in history but that's more involved.
This is actually maybe better behavior.
Test Plan: Clicked on the line link, dragged and dropped on it.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5172
Summary: This adds an option dust background for certain application designs, like Macro and Pholio to help make the list views pop more.
Test Plan: Reviewed Macro and Pholio.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5209
Summary: Simplify the look of the mobile menu, provide active states for the menu icons.
Test Plan: iOS Simulator and Chrome.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2426
Differential Revision: https://secure.phabricator.com/D5189
Summary:
actually just wanted to `pht()` the //Cancel// button.
includes `pht()`ed exceptions.
refrained from `pht()`-ing `phabricator_relative_date()` since I wasn't too sure about the effects.
Test Plan: visited all form elements that I remembered - looks reasonable
Reviewers: epriestley, chad, btrahan
Reviewed By: chad
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5175
Summary: Adds imageview (dark background) to Files and Macro.
Test Plan: Test a file and a macro, see darkness.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5131
Summary: Mostly applies a new call spacing rule; also a few things that have slipped through via pull requests and such
Test Plan: `find src/ -type f -name '*.php' | xargs -n16 arc lint --output summary --apply-patches`
Reviewers: chad
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D5002
Summary:
- In stack traces, a `,` should clearly be a `.`.
- In Calendar, a 'td' got swapped with a 'p' somewhere.
- In old-style transaction views, strlen() is no longer a sufficient test.
Test Plan:
- Verified stack traces render correctly.
- Verified calendar renders correctly.
- Verified Maniphest transactions with no comment no longer have a little empty div a few pixels high.
Reviewers: chad
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D4971
Summary: Converts the old table to a new item list, also added ability to link icons on right side of item stack.
Test Plan: Tested adding removing, sorting of projects both active and inactive.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4960
Summary: Sgrepped for `"=~/</"` and manually changed every HTML.
Test Plan: This doesn't work yet but it is hopefully one of the last diffs before Phabricator will be undoubtedly HTML safe.
Reviewers: epriestley
CC: aran, Korvin
Maniphest Tasks: T2432
Differential Revision: https://secure.phabricator.com/D4927
Summary: I'm too lazy to attaching them for diffs where they were introduced.
Test Plan:
/
/D1, wrote comment with code snippet
DarkConsole
commit detail, wrote comment
task detail, wrote comment
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2432
Differential Revision: https://secure.phabricator.com/D4911
Summary: `renderChildren()` now returns array which isn't ideal but I prefer it to having two methods.
Test Plan: None.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4910
Summary: They are same because render() returns safe HTML and raw strings are automatically escaped.
Test Plan: None.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4909
Summary:
This is pretty brutal and it adds some `phutil_safe_html()`.
But it is a big step in the right direction.
Test Plan: None.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2432
Differential Revision: https://secure.phabricator.com/D4905
Summary:
I like this abstraction better.
Result of `phutil_implode_html()` may be also used as a param of `hsprintf()`.
Test Plan: None.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4904
Summary:
This resolves lots of double escaping.
We changed most of `phutil_render_tag(, , $s)` to `phutil_tag(, , $s)` which means that `$s` is now auto-escaped.
Also `pht()` auto escapes if it gets `PhutilSafeHTML`.
Test Plan: None.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2432
Differential Revision: https://secure.phabricator.com/D4889
Summary:
- Separate the ideas of "requested" (explicit user request) vs "started" (user request or sampling).
- Move this code out of index.php into the XHProf stuff (general effort to make index.php smaller).
Test Plan:
Verified that profiling still works, and profiling extends to ajax requests.
Set sampling rate to 2, saw 50% samples.
Looked at database, saw sampling data populating properly.
Reviewers: vrana, nh
Reviewed By: vrana
CC: aran
Differential Revision: https://secure.phabricator.com/D4891
Summary:
Lots of killed `phutil_escape_html()`.
Done by searching for `AphrontTableView` and then `$rows` (usually) backwards.
Test Plan:
Looked at homepage.
echo id(new AphrontTableView(array(array('<'))))->render();
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4884
Summary:
Done by searching for `AphrontDialogView` and then `appendChild()`.
Also added some `pht()`.
Test Plan: None.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4882
Summary: Route all `$_SERVER['HTTP_...']` stuff through AphrontRequest (it would be nice to make this non-static, but the stack is a bit tangled right now...)
Test Plan: Verified CSRF and cascading profiling. `var_dump()`'d User-Agent and Referer and verified they are populated and returned correct values when accessed. Restarted server to trigger setup checks.
Reviewers: vrana
Reviewed By: vrana
CC: aran
Differential Revision: https://secure.phabricator.com/D4888
Summary: If a page is profiled, add an "X-Phabricator-Profiler" header to all Ajax requests, and profile those too.
Test Plan: Profiled a page, checked Darkconsole, saw profiles for everything.
Reviewers: vrana, btrahan
Reviewed By: vrana
CC: aran
Differential Revision: https://secure.phabricator.com/D4885
Summary: Done by searching for `AphrontErrorView` and then `appendChild()`.
Test Plan:
Looked at Commit Detail.
Looked at Revision Detail.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2432
Differential Revision: https://secure.phabricator.com/D4843
Summary: Resolves submit issues in IE7, scrollbars in IE7 and homepage layout issues in IE7 and IE8.
Test Plan: used IE7 and IE8. Logged in, bounced around. Checked Chrome as well.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2469, T2470
Differential Revision: https://secure.phabricator.com/D4853
Summary:
I wasn't able to reproduce the "recursion detected" in real web request but I saw lots of 1073741824 refcounts in `debug_zval_dump()` of $object.
I'm not sure how that happens.
Test Plan: D4807#4
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2432
Differential Revision: https://secure.phabricator.com/D4839
Summary:
mainly, this adds the image cropper - yay!
- also removes the file image from the handle stuff I added in V1. now we do all this crazy photo stuff.
Test Plan:
- uploaded a photo by dragging to header and noted 120 x 80 showed up on reload
- uploaded a photo by dragging to edit dialogue spot and noted 120 x 80 showed up on reload
- cropped a photo - noted it cropped right
- cropped a photo again and again and again - seems like it crops okay
Reviewers: epriestley, chad
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2418, T2399
Differential Revision: https://secure.phabricator.com/D4790
This is kind of gross, but breaking some menus right now which end up with double-`''` keys. The current meaning of setKey(null) is different from not calling it (it means `setKey('')`).
This should be fixed more reasonably but there's a lot of legacy cruft in PhabricatorSideNavFilterView.
Auditors: btrahan
Summary: do so via event engine. note different order now...
Test Plan: toggled "show beta applications" to off and noted that Conpherence disappeared. Otherwise noted that links showed.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2424
Differential Revision: https://secure.phabricator.com/D4708
Summary:
I attempted to test every interface. I probably missed some stuff, but I at least have some level of confidence that the `phutil_tag` branch is fairly stable.
Fixed these issues:
[1] Fixed a Herald issue with object links in transcripts. Some actions return
links; this was previously hard-coded.
[2] DarkConsole refactoring created an issue where the "`" event handler registered too many times.
[3] Fixed a bug where `strlen($value)` was being checked, but fields may now return array(). Possibly we should implement phutil_is_empty_html() or similar.
[4] Fixed a undefined variable issue for image edit transactions.
[5] Fixed an issue with rendering participant transactions. This required phutil_safe_html() because `pht()` can't handle `array()` for `%s`.
[6] Fixed an issue where feed was entirely overescaped by reverting an overly ambitious render_tag -> tag.
[7] Fixed an issue with strict tables and inserting `''` instead of `0` into an integer column.
[8] Fixed an issue where • was shown escaped.
[9] Fixed an issue where "no data" was overescaped.
[10] Fixed an issue with strict tables and inserting `''` instead of `0` into an integer column.
[11] Fixed an issue with strict tables and inserting `''`.
[12] Fixed an issue with missing space after ":" for mini panels.
Encountered (but did not fix) these issues:
[X1] "e" works incorrectly on comments you are not allowed to edit. Did not fix.
[X2] Coverage currently interacts incorrectly with "--everything" for Phutil tests.
Test Plan:
- Viewed Differential.
- Created a diff via copy/paste.
- Viewed standalone diff.
- Jumped to diff via changeset table.
- Created a revision.
- Updated revision.
- Added a comment.
- Edited revision dependencies.
- Edited revision tasks.
- Viewed MetaMTA transcripts.
- Viewed Herald transcripts [1].
- Downloaded raw diff.
- Flagged / unflagged revision.
- Added/edited/deleted inline comment.
- Collapsed/expanded file.
- Did show raw left.
- Did show raw right.
- Checked previews for available actions.
- Clicked remarkup buttons
- Used filetree view.
- Used keyboard: F, j, k, J, K, n, p, t, h, "?" [2] [X1].
- Created a meme.
- Uploaded a file via drag and drop.
- Viewed a revision with no reviewers.
- Viewed a revision with >100 files.
- Viewed various other revisions [3].
- Viewed an image diff.
- Added image diff inline comments.
- Viewed Maniphest.
- Ran various queries.
- Created task.
- Created similar task.
- Added comments to tasks.
- Ran custom query.
- Saved custom query.
- Edited custom queries.
- Drag-reordered tasks.
- Batch edited tasks.
- Exported tasks to excel.
- Looked at reports (issue in T2311 notwithstanding).
- Viewed Diffusion.
- Browsed Git, SVN, HG repositories.
- Looked at history, browse, change, commit views.
- Viewed audit.
- Performed various audit searches.
- Viewed Paste.
- Performed paste searches.
- Created, edited, forked paste.
- Viewed Phriction.
- Edited a page.
- Viewed edit history.
- Used search typeahead to search for user / application.
- Used search to search for text.
- Viewed Phame.
- Viewed Blog, Post.
- Viewed live post.
- Published/unpublished post.
- Previewed post.
- Viewed Pholio.
- Edited/commented mock.
- Viewed ponder.
- Viewed question.
- Added answer/comment.
- Viewed Diviner.
- Viewed Conpherence [4] [5].
- Made Conpherence updates.
- Viewed calendar.
- Created status.
- Viewed status.
- Viewed Feed [6].
- Viewed Projects.
- Viewed project detail.
- Edited project.
- Viewed Owners.
- Viewed package detail.
- Edited package [7].
- Viewed flags.
- Edited flag.
- Deleted flag.
- Viewed Herald.
- Viewed rules.
- Created rule.
- Edited rule.
- Viewed edit log.
- Viewed transcripts.
- Inspected a transcript.
- Viewed People.
- Viewed list.
- Administrated user.
- Checked username/delete stuff.
- Looked at create/import LDAP/activity logs.
- Looked at a user profile.
- Looked at user about page.
- Looked at Repositories.
- Edited repository.
- Edited arcanist project.
- Looked at daemons.
- Looked at all daemons [8].
- Viewed combined log.
- Looked at configuration.
- Edited configuration.
- Looked at setup issues [9].
- Looked at current settings.
- Looked at application list.
- Installed / uninstalled applications [10].
- Looked at mailing lists.
- Created a mailing list.
- Edited a mailing list.
- Looked at sent mail.
- Looked at received mail.
- Looked at send/receive tests.
- Looked at settings.
- Clicked through all the panels.
- Looked at slowvote.
- Created a slowvote [11].
- Voted in a slowvote.
- Looked at Macro.
- Created a macro.
- Edited a macro.
- Commented on a macro.
- Looked at Countdown.
- Created a Countdown.
- Looked at it.
- Looked at Drydock.
- Poked around a bit.
- Looked at Fact.
- Poked around a bit.
- Looked at files.
- Looked at a file.
- Uploaded a file.
- Looked at Conduit.
- Made a Conduit call.
- Looked at UIExamples.
- Looked at PHPAST.
- Looked at PHIDs.
- Looked at notification menu.
- Looked at notification detail.
- Logged out.
- Logged in.
- Looked at homepage [12].
- Ran `arc unit --everything --no-coverage` [X2].
Reviewers: vrana, btrahan
Reviewed By: vrana
CC: aran
Maniphest Tasks: T2432
Differential Revision: https://secure.phabricator.com/D4807
Summary: This is the most requested feature in FB by far.
Test Plan:
Toggled, verified that data are saved.
Reloaded, toggled, toggled, toggled, verified that data are saved.
Reloaded.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4812
Summary: Remove css class that was setting static width
Test Plan: Loaded user profile edit page, stretched browser around, saw that the text was happy
Reviewers: epriestley, btrahan
Reviewed By: btrahan
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4806
Summary:
Convert a final few `render_tag()` calls to `tag()` calls. This leaves us with 36 calls:
- 9 are in Conpherence, and will be converted after merging. Using Views makes the most sense here, to get access to renderHTMLView() and such.
- 2 are the definition and its library map entry.
- 3 are in the documentation.
- I believe the remaining 22 are too difficult to convert pre-merge. About half are in code which is slated for destruction; the other half are in the base implementations of very common classes (like PhabricatorStandardPageView) and can only be converted by converting the entire codebase.
My plan at this point is:
- Test the branch thoroughly.
- Merge to master.
- Over time, resolve the remaining issues: lint means things shouldn't get any worse.
Test Plan: See inlines.
Reviewers: vrana
Reviewed By: vrana
CC: aran, edward
Maniphest Tasks: T2432
Differential Revision: https://secure.phabricator.com/D4802
Summary:
Make `PhabricatorMenuView` more flexible, so callers can add items to the beginning/end/middle.
In particular, this allows event handlers to receive a $menu and call `addMenuItemToLabel('activity', ...)` or similar, for D4708.
Test Plan: Unit tests. Browsed site. Home page, Conpherence, and other pages with menus look correct.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D4792
Summary: Refactor options related to verbose error reporting and forcing disk reads into a single developer option.
Test Plan: Run Phabricator with the developer-mode option set and check that errors print stack traces, static assets are always reloaded, etc.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4780
Summary:
- Crumbs is straightforward.
- Maniphest does a lot of string construction which isn't i18n or safehtml aware. I cheated under the theory that we'll resolve this properly in {T2217}.
Test Plan: Looked at crumbs and Maniphest.
Reviewers: vrana
Reviewed By: vrana
CC: aran
Maniphest Tasks: T2432
Differential Revision: https://secure.phabricator.com/D4748
Summary: Convert most phabricator_render_form callsites. In the case of the "headsup view", it converts it by deleting the element entirely (this is the very old Maniphest/Differential header which we no longer use).
Test Plan: Poked around a bit.
Reviewers: vrana
Reviewed By: vrana
CC: aran
Maniphest Tasks: T2432
Differential Revision: https://secure.phabricator.com/D4726
Summary: This adds a new menu item, TYPEBUTTON, for use in Conpherence and maybe others over time. Note I need to add icon support, but I'll make them later tonight. I also changed the front facing names to 'Conversations' which is way more natural. Basically, Pholio has Mocks, Differential has Diffs, Conpherence has Conversations.
Test Plan: Tested Conpherence on mobile and desktop.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2430
Differential Revision: https://secure.phabricator.com/D4691
Summary:
This accomplishes three major goals:
# Fixes phutil_render_tag -> phutil_tag callsites in DarkConsole.
# Moves the Ajax request log to a new panel on the left. This panel (and the tabs panel) get scrollbars when they get large, instead of making the page constantly scroll down.
# Loads the panel content over ajax, instead of dumping it into the page body / ajax response body. I've been planning to do this for about 3 years, which is why the plugins are architected the way they are. This should make debugging easier by making response bodies not be 50%+ darkconsole stuff.
Additionally, load the plugins dynamically (the old method predates library maps and PhutilSymbolLoader).
Test Plan:
{F30675}
- Switched between requests and tabs, reloaded page, saw same tab.
- Used "analyze queries", "profile page", triggered errors.
- Verified page does not load anything by default if dark console is closed with Charles.
- Generally banged on it a bit.
Reviewers: vrana, btrahan, chad
Reviewed By: vrana
CC: aran
Maniphest Tasks: T2432
Differential Revision: https://secure.phabricator.com/D4692
Summary:
I broke this a bit a few revisions ago by making `phabricator_render_csrf_magic()` return array instead of string without actually grepping for callsites.
Instead of building a form in JS, build it in PHP and just change its action in JS. This is simpler and doesn't require us to expose CSRF magic in more places.
This change triggered a rather subtle bug: if we rendered a normal form on the page (and thus installed the "disable forms when submitted" behavior), the download button would incorrectly disable after being clicked. This doesn't currently happen in Files because we don't put a form on the same page as the download button.
Instead, make the "disable" behavior check if the form is downloading stuff, and not disable stuff if it is. Then mark the lightbox and Files form as both download forms.
Test Plan: Used lightbox; used Files. Verified download buttons download the right stuff and behave correctly. Grepped for other download forms, but all the other places where we write "download" don't appear to actually be download links.
Reviewers: vrana, btrahan
Reviewed By: vrana
CC: aran
Maniphest Tasks: T2432
Differential Revision: https://secure.phabricator.com/D4685
Summary: Some content might be broken but it's hard to test since JS/Ajax is also a bit broken.
Test Plan: Looked at timeline examples.
Reviewers: vrana
Reviewed By: vrana
CC: aran
Maniphest Tasks: T2432
Differential Revision: https://secure.phabricator.com/D4683
Summary: This works after pht() + html got sorted out.
Test Plan: Looked at some object attribute lists.
Reviewers: vrana
Reviewed By: vrana
CC: aran
Maniphest Tasks: T2432
Differential Revision: https://secure.phabricator.com/D4645
Summary: Notification menu shows a loading message before the menu is populated with the actual response.
Test Plan: Checked by having the function sleep between getting the response and populating the menu.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4696
Summary: decent title. Stylistically its probably a bit rough. Also, I think @chad describes an even hotter workflow in T2418. Note this removes the "default image" thing which I don't think makes sense conceptually since by default the image changes to who replied last...
Test Plan: uploaded an image - worked. uploaded a txt file - got ugly errors that file was not supported.
Reviewers: epriestley, chad
Reviewed By: chad
CC: aran, Korvin
Maniphest Tasks: T2417
Differential Revision: https://secure.phabricator.com/D4668
Summary: Adds notification icons for Conpherence and re-writes the CSS a bit for the new icons and states. I removed the background bubble here and went straight CSS. I also seem to have a JS error and the notification menu doesn't display, but I'm tired and wanted to look at this in Differential. Will update after JS fix.
Test Plan: Turned on notification numbers and conpherence numbers, turned them off. Made them big. Checked FF and IE.
Reviewers: epriestley, btrahan
Reviewed By: btrahan
CC: aran, Korvin
Maniphest Tasks: T2415
Differential Revision: https://secure.phabricator.com/D4666
Summary: Proper fix is to do some layout work in Diffusion. Short of that, make this escape properly.
Test Plan: Viewed various crumbs, no more overescaping for non-diffusion crumbs.
Reviewers: vrana
Reviewed By: vrana
CC: aran
Differential Revision: https://secure.phabricator.com/D4641
Summary: Fixes various array_combine() warnings for PHP < 5.4
Test Plan: lint/unit/grep
Reviewers: btrahan, vrana, chad
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D4660
Summary:
- Implement AphrontView->renderHTMLView(), which works like AphrontView->renderSingleView() but always return PhutilSafeHTML.
Test Plan: Action list view no longer as broken.
Reviewers: vrana
Reviewed By: vrana
CC: aran
Differential Revision: https://secure.phabricator.com/D4642
Summary:
- Implements `javelin_tag()`, which is `javelin_render_tag()` on top of `phutil_tag()` instead of `phutil_render_tag()`.
- Manually converts all or almost all of the trivial callsites.
Test Plan:
- Site does not seem any more broken than before.
Reviewers: vrana
Reviewed By: vrana
CC: aran
Differential Revision: https://secure.phabricator.com/D4639
Summary:
- Grepped for phutil_render_tag().
- Fixed some easy ones.
Test Plan:
- Browsed around; site didn't seem more broken than it was before.
Reviewers: vrana
Reviewed By: vrana
CC: aran
Differential Revision: https://secure.phabricator.com/D4638
Summary:
This adds a new method for rendering the object list as a stackable set of items. Good for certain renderings like Config.
u
Test Plan: Review list on iOS, Chrome, FF.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4637
Summary:
Created with spatch:
lang=diff
- phutil_render_tag
+ phutil_tag
(X, Y, pht('...'))
The searched for `<` and `&` by sgrep.
Test Plan: Loaded homepage.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4504
Summary:
Created with spatch:
lang=diff
- phutil_render_tag
+ phutil_tag
(X, Y, '...')
Then searched for `&` and `<` in the output and replaced them.
Test Plan: Loaded homepage.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4503
Summary: Fixes a Conpherence fatal when going to /conpherence/update/1
Test Plan: Successfully rendered the edit form and saved it.
Reviewers: epriestley, btrahan
Reviewed By: btrahan
CC: aran, Korvin
Maniphest Tasks: T2402
Differential Revision: https://secure.phabricator.com/D4634
Summary: it's ugly. but it works. basically. See T2399 for a roughly prioritized list of what still needs to happen.
Test Plan:
- created a conpherence with myself from my profile
- created a conpherence with myself from "new conpherence"
- created a conphernece with another from "new conpherence"
- created a conpherence with several others
- created a conpherence with files in the initial post
- verified files via comment text ("{F232} is awesome!") and via traditional attach
- edited a conpherence image
- verified it showed up in the header and in the conpherence menu on the left
- edited a conpherence title
- verified it showed up in the header and in the conpherence menu on the right
- verified each widget showed up when clicked and displayed the proper data
- calendar being an exception since it sucks so hard right now.
Reviewers: epriestley, chad
Reviewed By: epriestley
CC: aran, epriestley, chad, codeblock, Korvin
Maniphest Tasks: T2301
Differential Revision: https://secure.phabricator.com/D4620
Summary: This needs some tweaks but I'll follow up with @DeedyDas in T2353.
Test Plan: So many memes.
Reviewers: chad, btrahan, DeedyDas
Reviewed By: chad
CC: aran
Maniphest Tasks: T2353
Differential Revision: https://secure.phabricator.com/D4616
Summary: This wraps tables in a scrollable area when viewports are too narrow for the content.
Test Plan: Tested table layout in Chrome and iOS. Config was easiest table to test on mobile.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4586
Summary: Fixes T2330. Deletes the last traces of the footer.
Test Plan: Looked at several pages, saw no footer. Grepped for footer function and CSS class.
Reviewers: DeedyDas, chad
Reviewed By: chad
CC: aran
Maniphest Tasks: T2330
Differential Revision: https://secure.phabricator.com/D4554
Summary: Just removed the link and created a new field under preferences. Now the setting is under Display Preferences.
Test Plan: Enablied/Disabled dark console to see if it works.
Reviewers: epriestley
Reviewed By: epriestley
CC: irinav, aran, Korvin
Maniphest Tasks: T2344
Differential Revision: https://secure.phabricator.com/D4549
Conflicts:
src/view/page/PhabricatorStandardPageView.php
Summary: Fixed T2349
Test Plan:
Could not visibly see version at footer any more. Appeared in the top of /config.
Does not appear as a config option in /config.
Reviewers: epriestley
CC: aran, Korvin
Maniphest Tasks: T2349
Differential Revision: https://secure.phabricator.com/D4539
Summary:
Migrate to the new hotness. Also:
- Remove a string test, which is now impossible since the config will repair itself and raise a type error.
- Restore the header even in /config/ -- this check is kind of hacky and it feels a bit more natural now that it's above the menu.
Test Plan: Set my local disk path to something invalid, verified I got a setup error.
Reviewers: btrahan, vrana
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2228
Differential Revision: https://secure.phabricator.com/D4497
Summary: Removed the 'Report a Bug' link from the footer
Test Plan: Tried looking through all the phabricator pages but didn't quite get around to it.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4528
Summary: We now issue only valid setup warnings, so we can let administrators know when we detect problems.
Test Plan:
Banner:
{F29568}
Created a fake issue; saw banner. No banner inside /config/. Resovled the issue, banner went away.
Reviewers: btrahan, chad
Reviewed By: chad
CC: aran
Maniphest Tasks: T2221
Differential Revision: https://secure.phabricator.com/D4493
Summary: Some views expose some/most/all of the basics (id, class, sigil, metadata, etc); let them extend a dedicated view to get it for free.
Test Plan: Viewed home page, paste, mobile dropdown, etc.
Reviewers: btrahan, chad
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D4440
Summary:
See discussion in D4438. Allows users to customize application tiles, and implements generally reasonable defaults so they hopefully won't.
Sizes are "invisible" (internal only, used to hide admin apps from non-admins), "hidden" (hide by default, show after clicking "Show More Applications"), "show" (show a small square tile) and "full" (show a full-width tile with subtitle).
Test Plan:
Default view for a non-admin:
{F29375}
Adjusted settings, hidden:
{F29373}
Adjusted settings, shown:
{F29374}
Reviewers: chad, btrahan
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D4439
Summary:
No fancy-pants smarty stuff yet, but merges /applications/ and the awful application buttons into the dark navigation.
Hover state is maybe a little weird.
Test Plan: {F29324}
Reviewers: chad, btrahan
Reviewed By: btrahan
CC: aran, btrahan, codeblock
Differential Revision: https://secure.phabricator.com/D4431
Summary:
Maniphest and Owners still have green ListFilter buttons, which have looked awkward for a while and are extra-awkward after D4447. Move them into crumbs and remove the ability of ListFilter to support buttons.
The actual implementation can be simplified too now.
Test Plan: Looked at Owners, Maniphest. Clicked create buttons. Looked at UIExample.
Reviewers: chad
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D4451
Summary: This starts the content over 7 px instead of overlapping the drag bar on the side menu.
Test Plan: Tested it in Chrome with scrollbar
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2322
Differential Revision: https://secure.phabricator.com/D4449
Summary: Still working through basic re-design. Adds the ability to re-use panel view without the background.
Test Plan: Viewed Diffusion and Differential in Chrome, FF, Safari.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4430
Summary: If there are items (like labels) without keys, we incorrectly select the first item with no key in response to a `selectFilter(null)` call.
Test Plan: Called `selectFilter(null)`, didn't get a selected label.
Reviewers: chad
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D4428
Summary: This removes all calls to addSpacer and the method. We were applying it inconsistently and it was causing spacing issues with redesigning the sidenav. My feeling is we can recreate the space in CSS if the design dictates, which would apply it consistently.
Test Plan: Go to Applications, click on every application.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4420
Summary:
My claims that this worked on D4183 were incorrect:
- Make it work.
- Then use it.
- Also make cancel URI not cancel to the same page.
Test Plan: Clicked "Create Document", got a workflow on-page dialog instead of a page transition.
Reviewers: codeblock
Reviewed By: codeblock
CC: aran
Differential Revision: https://secure.phabricator.com/D4403
Summary: Move all navs to use the newer-style, darker, textured look. I'm //pretty// sure this doesn't break anything.
Test Plan: Looked at a bunch of apps.
Reviewers: btrahan, chad
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D4376
Summary:
- Adds the rest of the group as per T2255.
- Adds a pht() around the `$developer_warning` in `PhabricatorStandardPageView`.
Test Plan:
- Viewed new config options.
- Triggered a fake warning to make sure I didn't break error callouts.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2255
Differential Revision: https://secure.phabricator.com/D4333
Summary: This is obsolete with the 'device' => true stuff, which is more granular and generally better.
Test Plan: grep
Reviewers: btrahan, codeblock
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2255
Differential Revision: https://secure.phabricator.com/D4309
Summary: We have enough z-index rules that they're fairly hard to visualize with "git grep". Consolidate them. Then fix T2253 (missing z-index on left menu background).
Test Plan: Made a Differential window really narrow, then scrolled it horizontally.
Reviewers: btrahan, chad, ender
Reviewed By: chad
CC: aran
Maniphest Tasks: T2253
Differential Revision: https://secure.phabricator.com/D4302
Summary:
The placeholder property remains null despite it is set to a custom
value.
Test Plan:
Use a custom placeholder in `AphrontFormTokenizerControl`
```
id(new AphrontFormTokenizerControl())
->setPlaceholder('My custom placeholder...')
->setDatasource('/typeahead/common/projects/');
```
The placeholder should be set to "My custom placeholder..."
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4297
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: This is used in every other view.
Test Plan: Browsed around.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4248
Summary: Continue work started at D3601.
Test Plan:
Commented declaration `AphrontController::$request`, saw exception.
Brought it back, didn't see exception.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4233
Summary: This is probably not the most useful app to have work on mobile, but get the log view to do something fairly sensible.
Test Plan: Looked at all Drydock views in mobile.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2015
Differential Revision: https://secure.phabricator.com/D4224
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: 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:
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: 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:
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: 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: 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: 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:
- 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:
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: 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:
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:
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:
- 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:
This is still rough and not completely accurate to the mocks (and the mobile view is quite crude and mostly just "hey this technically works"), but I want to build Pholio on top of it rather than building it on something else and then swapping it out later and the API is reasonable enough.
This should probably be called `PhabricatorTransactionView` but we already have one of those. I might juggle the names in a future diff.
Test Plan:
Desktop
{F22352}
Mobile
{F22351}
Reviewers: chad, btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2097
Differential Revision: https://secure.phabricator.com/D3833
Summary:
I'm not going to land this until it's a bit more fleshed out since it would just confuse users, but this is probably more reviewable as a few diffs adding a couple features than one ULTRA-diff adding everything. Implement application basics for Pholio. This does more or less nothing, but adds storage, subscribe, flag, markup, indexing, query basics, PHIDs, handle loads, a couple of realy really basic controllers, etc.
Basic hierarchy is:
- **Moleskine**: Top-level object like a Differential Revision, like "Ponder Feed Ideas".
- **Image**: Each Moleskine has one or more images, like the unexpanded / expanded / mobile / empty states of feed.
- **Transaction**: Comment or edit, like Maniphest. I generally want to move most apps to a transaction model so we can log edits.
- **PixelComment**: Equivalent of an inline comment.
Test Plan: Created a fake object and viewed it.
Reviewers: btrahan, chad
Reviewed By: btrahan
CC: aran, davidreuss
Maniphest Tasks: T2097
Differential Revision: https://secure.phabricator.com/D3817
Summary:
- For Drydock, I want to add section headers to separate user-defined attributes from global attributes.
- Some day for Differential, I want to add "Summary" and "Test Plan" section headers.
- Clean up some stuff a bit; drop the multiple APIs for setting text content. Explicitly disallow appendChild().
- Build out the UIExample a bit.
Test Plan:
{F24821}
{F24822}
Reviewers: chad, btrahan
Reviewed By: chad
CC: aran
Maniphest Tasks: T2015
Differential Revision: https://secure.phabricator.com/D4000
Summary: defensive program wins all battles
Test Plan: lightbox still worked
Reviewers: vrana, epriestley
Reviewed By: vrana
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D3975
Summary:
This commit doesn't change license of any file. It just makes the license implicit (inherited from LICENSE file in the root directory).
We are removing the headers for these reasons:
- It wastes space in editors, less code is visible in editor upon opening a file.
- It brings noise to diff of the first change of any file every year.
- It confuses Git file copy detection when creating small files.
- We don't have an explicit license header in other files (JS, CSS, images, documentation).
- Using license header in every file is not obligatory: http://www.apache.org/dev/apply-license.html#new.
This change is approved by Alma Chao (Lead Open Source and IP Counsel at Facebook).
Test Plan: Verified that the license survived only in LICENSE file and that it didn't modify externals.
Reviewers: epriestley, davidrecordon
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2035
Differential Revision: https://secure.phabricator.com/D3886
Summary: Make mobile-friendly and provide UI to cancel/retry tasks. Remove display of task data to arbitrary users, as it may be sensitive.
Test Plan:
{F22502}
{F22503}
{F22504}
{F22505}
{F22506}
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2015
Differential Revision: https://secure.phabricator.com/D3854
Summary:
Minor tweaks to lightboxes.
- With "position: fixed;", we don't need to do any of the scroll/resize stuff. Just remove it.
- Make the lightbox go over the menu bar -- was it intentional that it wasn't?
- Make 'jx-mask' use "position: fixed;" too.
- Add a loading indicator.
- In Differential/Maniphest/etc, a preview may bring in an image but won't bring in the CSS we need. The "real" fix is to ship CSS/JS with ajax, but that's really hard -- fake it by pulling in the right CSS any time we render a remarkup area.
I'm going to do a couple of other tweaks here but need to update JX.Mask.
Test Plan: Verified behavior is reasonable in Safari, Firefox, Chrome with multiple images / scroll / previews / resize.
Reviewers: btrahan, vrana
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T1896
Differential Revision: https://secure.phabricator.com/D3795
Summary:
Provides a simple way for policy-aware queries to pre-filter results without needing to maintain separate cursors, and fixes a bunch of filter-related edge cases.
- For reverse-paged cursor queries, we previously reversed each individual set of results. If the final result set is built out of multiple pages, it's in the wrong order overall, with each page in the correct order in sequence. Instead, reverse everything at the end. This also simplifies construction of queries.
- `AphrontCursorPagerView` would always render a "<< First" link when paging backward, even if we were on the first page of results.
- Add a filtering hook to let queries perform in-application pre-policy filtering as simply as possible (i.e., without maintaing their own cursors over the result sets).
Test Plan: Made feed randomly prefilter half the results, and paged forward and backward. Observed correct result ordering, pagination, and next/previous links.
Reviewers: btrahan, vrana
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D3787
Summary:
Make the example page a little more useful by showing available icons.
Also replace the "new" image, it had a little arrow which I thought was a "+". Use the one with a "+".
Test Plan: {F21966}
Reviewers: btrahan, vrana
Reviewed By: vrana
CC: aran
Differential Revision: https://secure.phabricator.com/D3794
Summary:
See discussion in T404. Basically, the problem with date-only controls is that they may behave unpredictably in the presence of timezones. When you say "This needs to be done by Oct 23", you probably mean "Oct 23 5PM PST" or something like that, but someone in China may see the "Oct 24" and hit the deadline in good faith but be 10 hours too late. T404 has more discussion and examples. There are ways to fake this, but they get more complicated if the guy in China needs to move the date forward 24 hours.
I think the best solution to this is to not have date-only controls, and always display the time. This makes it absolutley unambiguous what something means, because the guy in the US will set "Oct 23 5PM" and the guy in China will see that accurately in local time.
The downside is that it's slightly more visual clutter and work for the user to specify things precisely, but I added some hints (start/end of day, start/end of business) that will hopefully let us pick the right default in most cases.
Test Plan:
Set some dates.
{F21956}
This has a couple of edge case issues on resize and some not-so-edge-case issues on mobile, but should be good to build T407 on without API changes.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T404, T407
Differential Revision: https://secure.phabricator.com/D3793
Summary:
images attached to maniphest tasks and mentioned in remarkup anywhere now invoke a lightbox control that lets the user page through all the images.
lightbox includes a download button, next / prev buttons, and if we're not at the tippy toppy of hte page an "X" or close button.
we also respond to left, right, and esc for navigating.
next time we should get non-images working in here...!
Test Plan:
played with maniphest - looks good
made comments with images. looks good.
made sure multiple image comments worked.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin, chad
Maniphest Tasks: T1896
Differential Revision: https://secure.phabricator.com/D3705
Summary: Can you please pick up the icon for it and regenerate sprites?
Test Plan:
Selected text, clicked on it.
Unselected text, clicked on it.
Reviewers: epriestley, chad
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D3730
Summary:
For Phame, we really want more than a chromeless page -- it shouldn't have Javelin or Phabricator styles on it.
Pull the bare infrastructure out of StandardPageView and make it available as BarePageView.
Test Plan: Viewed bare page in UIExamples, various non-bare pages.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D3711
Summary: These haven't had any effect since the redesign, and we're moving to a more granular policy model so it probably doesn't make sense to ever restore them.
Test Plan: Grepped for "admin" and removed all relevant code.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D3710
Summary:
- Better icons and action order.
- "Move Post" action.
- (Bugfix) Allow multiple blogs to be set to not having custom domains.
- Make "Write Post" skip the "select a blog" step when coming from a blog view.
- Sort blog list on "Write Post".
- Show messages when a post is a draft or not on a blog.
Test Plan: Created posts, blogs, moved posts, preview/live'd posts, etc.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T1373
Differential Revision: https://secure.phabricator.com/D3708
Summary:
Some objects (like PhamePost and ManiphestTask) have a block of text/remarkup which serves as a description or core piece of content for the object.
Accommodate this in PhabricatorPropertyListView.
(This is primarily to let me do a reasonable first pass on this in Phame.)
Test Plan: Made example, will attach screenshot.
Reviewers: btrahan, chad
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T1373
Differential Revision: https://secure.phabricator.com/D3699
Summary: Make these always work. Notably, this makes them work in Maniphest. Previously this was at odds with stuff fixed in D3651.
Test Plan: Dragged and dropped files into Remarkup in Maniphest.
Reviewers: btrahan, vrana
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D3652
Summary:
Currently, in Maniphest, if you drag-and-drop a file it always attaches. Instead, I want you to have two options:
- Drag and drop to the attachment area to attach; or
- drag and drop to the Remarkup panel to upload + inline.
For the first step, make the input have a clear drop target instead of it being the entire panel.
Test Plan: Attached files in Create Task, task view, meta mta send test.
Reviewers: btrahan, vrana
Reviewed By: vrana
CC: aran
Differential Revision: https://secure.phabricator.com/D3651
Summary:
- I made a "?" icon for help/reference.
- The `<>` icon was slightly too wide so I carved it down to 14x14.
- All the icons are in `/Phabriactor/remarkup_icon_sources.psd` if you want to tweak anything.
- Tooltips don't look like the mock but I'll tackle those separately.
- Removed strikethrough.
- Removed tag/image/text size for now since they don't have reasonable JS implementations yet.
- I think everything else is accurate to the mock.
Test Plan:
Normal state:
{F20621, size=full}
Hover + Click states:
{F20622, size=full}
Clicked state:
{F20620, size=full}
Reviewers: chad
Reviewed By: chad
CC: aran
Maniphest Tasks: T1848
Differential Revision: https://secure.phabricator.com/D3650
Summary: @chad, can you do the icon sheets based on 1.6? We're using a few icons not present in 1.5. I put the 1.6 "pro" source on Dropbox.
Test Plan:
Nav hover and selected states:
{F20598}
Launch hover state:
{F20596}
Reviewers: chad
Reviewed By: chad
CC: aran
Maniphest Tasks: T1856
Differential Revision: https://secure.phabricator.com/D3649
Summary: This is sort of silly as-is, but automatically exposes flagging and will give subscribe/unsubscribe and "Subscribers" a place to plug into shortly. For context, see D3637 and T1808.
Test Plan: {F20550}
Reviewers: pieter, btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T1808
Differential Revision: https://secure.phabricator.com/D3641
Summary: So they're maybe a little easier to deal with? I'm going to take this formally to "plz @chad plz help" land.
Test Plan: {F20329}
Reviewers: btrahan, vrana, chad
Reviewed By: vrana
CC: aran
Differential Revision: https://secure.phabricator.com/D3609
Summary:
I make this error quite often: I forget to declare a property I am writing to or I make a typo in it.
PHP implicitly creates a public property which I don't like.
I would much rather see a linter warning me against this than this runtime check but writing it is very difficult:
- We need to explore all parents of the class we are checking.
- It is even possible that children will declare that property but it's OK to treat this as error anyway.
- We can extend also builtin or external classes.
- It's somewhat doable for `$this` but even more complex for any `$obj` because we don't know the class of it.
This should catch significant part of these errors and I'm fine with that.
I don't plan escalating to exception because this error is not fatal and should not stop the application from working.
Test Plan: Loaded homepage, checked log.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D3601
Summary: In some cases, we want an action item (like "Subscribe") to effect a write that needs a CSRF check. Allow such items to render as forms so they gracefully degrade if JS is FUBAR'd. D3499 has a specific example.
Test Plan: Loaded new UI example page, clicked all the actions.
Reviewers: btrahan, vrana, avivey
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D3596
Summary: Unblocker for D3547. Adds markup assist UI (buttons which generate remarkup for you -- not WYSIWYG) to Remarkup text areas.
Test Plan: See screenshot. Clicked the buttons a bunch with selected/unselcted text. Results seem broadly reasonable.
Reviewers: btrahan, vrana, teisenbe
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T337
Differential Revision: https://secure.phabricator.com/D3594
Summary: Moves toward unblocking D3547. Use a pinboard/album view to show image macros. Modernize and make (mostly) responsive.
Test Plan: Will attach screenshots.
Reviewers: btrahan, vrana, chad, teisenbe
Reviewed By: vrana
CC: aran
Maniphest Tasks: T175
Differential Revision: https://secure.phabricator.com/D3576
Summary:
Use the new `PhabricatorObjectItemListView` in Ponder so it works with the new UI. It will also get some features like flags "for free" in the future.
This removes the pager; I'll restore it in the next diff.
Test Plan: Looked at feed.
Reviewers: pieter, starruler
Reviewed By: pieter
CC: aran, chad
Maniphest Tasks: T1644
Differential Revision: https://secure.phabricator.com/D3507
Summary: ...also makes Maniphest Task Edit Controller use this when its not appropriate to upsell email.
Test Plan: played around with each tool and verified the Remarkup reference was present
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T1756
Differential Revision: https://secure.phabricator.com/D3468
Summary: Does what it says on the tin
Test Plan: Viewed ponder question, expanded link, added comment
Reviewers: pieter, epriestley
Reviewed By: pieter
CC: vrana, aran, Korvin
Maniphest Tasks: T1775
Differential Revision: https://secure.phabricator.com/D3485
Summary:
- Renames `PhabricatorPolicyQuery` to `PhabricatorPolicyAwareQuery` (a query which respects policy settings).
- Introduces `PhabricatorPolicyQuery`, which loads available policies (e.g., "member of project X").
- Introduces `PhabricatorPolicy`, which describes a policy.
- Allows projects to be set as policies.
- Allows Paste policies to be edited.
- Covers crazy cases where you make projects depend on themselves or each other because you are a dastardly villan.
Test Plan: Set paste and project policies, including crazy policies like A -> B -> A, A -> A, etc.
Reviewers: vrana, btrahan
Reviewed By: vrana
CC: aran
Maniphest Tasks: T603
Differential Revision: https://secure.phabricator.com/D3476
Summary:
This is the last Paste UI element that doesn't work properly on tablets/phones. Make it flexible.
Also add empty states to Paste.
Test Plan: Viewed various errors, and `/uiexample/errors/`.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D3429
Summary: Replaces the full names after D3413.
Test Plan: See screenshot.
Reviewers: vrana
Reviewed By: vrana
CC: aran
Differential Revision: https://secure.phabricator.com/D3414
Summary:
There is basically no reason for anyone to ever use the uncollapsed mode for more than the first 2 minutes of using the tool.
Delete all code related to collapse/expand.
(I'm going to add tooltips next.)
Also move the drag bar a few pixels to the right, so it does not overlap with the scrollbar on the "local" nav if there is one.
Test Plan: Viewed in desktop/tablet/phone modes.
Reviewers: vrana
Reviewed By: vrana
CC: aran
Differential Revision: https://secure.phabricator.com/D3413
Summary:
See some discussion in D1673.
- There's a concrete (if minor) problem with this in Firefox with wrapping search.
- People complain about how we're stealing all their pixels.
- There isn't much of a functional purpose to it since all the operations are fairly rare.
- This addresses the aesthetic purpose of the fixed-position nav (not making the side nav ugly) by making the side nav scroll up 44px and then stop.
Test Plan: Scrolled in desktop, tablet modes.
Reviewers: vrana
Reviewed By: vrana
CC: aran
Differential Revision: https://secure.phabricator.com/D3412
Summary:
Ball is more obvious and visible than I thought.
Delete the status word and display until date in title.
Also display the until date in revision list.
Also display near future dates with DoW instead of year.
Test Plan: Displayed revision and revision list with away reviewers.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D3407
Summary:
This does a few things:
- Allows you to flag pastes. This is straightforward.
- Allows Applications to register event listeners.
- Makes object action lists emit a 'didrenderactions' event, so other applications can add more actions. The Flags application injects its action in this way. This should generally make it much easier to add actions to objects when we add new applications, with less code duplication and better modularity. We have a really hacky version of this in Differential that I want to get rid of in lieu of this more general approach. I'm going to make object lists do the same thing, so any application can jump in and add stuff.
Test Plan: Flagged and unflagged pastes. Viewed home page, differential, flags list.
Reviewers: vrana, btrahan
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D3377
Summary: Permits the name and langauge of a paste to be edited. This will eventually allow the visibility policy to be edited as well.
Test Plan: Edited name/langauge of some pastes. Tried to edit a paste I didn't own, was harshly rebuffed.
Reviewers: vrana, btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T1690
Differential Revision: https://secure.phabricator.com/D3376
Summary:
We have this hybrid "create / last few pastes" landing screen right now but I ~never use the list at the bottom and it makes the controller kind of complicated. I want to let you edit pastes too, and this generally simplifies things.
Also makes the textarea monospaced and cleans up the fork logic a bit.
Test Plan: Created, forked pastes. Viewed paste lists. Viewed pastes.
Reviewers: vrana, btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T1690
Differential Revision: https://secure.phabricator.com/D3375
Summary: It seemed like a good idea at the time.
Test Plan: Uh huh.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D3352
Summary:
Adds a flexible navigation menu to diffs that shows you your current position in the diff.
Anticipating some "this is the best thing ever" and some "this is the wosrt thing ever" on this, but let's see how much pushback we get? It seems pretty good to me.
Test Plan: Will attach screenshots.
Reviewers: vrana, btrahan
Reviewed By: vrana
CC: aran
Maniphest Tasks: T1633, T1591
Differential Revision: https://secure.phabricator.com/D3355
Summary: I had no idea what checkered is.
Test Plan: Flagged revision, flagged task.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D3346
Summary:
This is another experiment for reducing reviewers response time.
I stole the idea (and colors) from [[ http://www.reviewboard.org/media/screenshots/2009/02/02/dashboard.png | ReviewBoard ]].
I actually quite like it (except when everything is red) and I can image that people will review just to have better color balance.
The code is not production ready for these reasons:
- We load holidays again and again for each revision. I couldn't cache it to static variable because it could persist multiple requests, right?
- I don't know how to expand height to the whole cell (I'm really bad in CSS).
- CSS rules are probably in wrong file.
- We probably want to use different colors.
This is how it looks:
{F16406}
Test Plan: Displayed revision list.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D3190
Summary:
I need to visit Phabricator homepage (usually to read the docs) quite often.
This is also kind of a signature.
Test Plan: Clicked it.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D3335
Summary:
This is the first time I've ever had CSS actually work like it promises it does (i.e., markup the "right" way and then you don't have to change the markup later).
Since I laboriously laid this whole thing out with <divs> originally, I was able to just override some of the styles and make the layout reasonable for devices.
The only differences for existing forms are:
- No colon after labels (looks cleaner anyway).
- Non-error required text is no longer a red star but a the grey word "Required" (this is clearer).
Test Plan:
Viewed paste form on a phone.
Viewed ~20 other forms on the site to verify that I didn't break anything.
Reviewers: btrahan, chad, vrana
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T1569
Differential Revision: https://secure.phabricator.com/D3298
Summary:
- Add a PhabricatorApplication.
- Make most of the views work well on tablets / phones. The actual "Create" form doesn't, but everything else is good -- need to make device-friendly form layouts before I can do the form.
Test Plan: Will attach screenshots.
Reviewers: btrahan, chad, vrana
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T1569
Differential Revision: https://secure.phabricator.com/D3293
Summary:
See D3277, D3278.
- Sprite all the menu icons.
- Delete the unsprited versions.
- Notification bolt now uses the same style as everything else.
Test Plan: Looked at page, hovered, clicked things.
Reviewers: btrahan, chad, vrana
Reviewed By: vrana
CC: aran
Maniphest Tasks: T1569
Differential Revision: https://secure.phabricator.com/D3279
Summary:
- Add getHelpURI() to PhabricatorApplication for application user guides.
- Add a new "help" icon menu item and skeletal Diviner application.
- Move help tabs to Applications where they exist, document the other ones that don't exist yet.
- Grep for all tab-related stuff and delete it.
Test Plan: Clicked "help" for some apps. Clicked around randomly in a bunch of other apps.
Reviewers: btrahan, vrana
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D3267
