Summary:
I want to add comments to commits, and they should obviously share code with the
nearly-identical comments in Maniphest and Differential. Unify code/style as
much as possible.
This program made possible by a generous grant from D1513.
Test Plan:
- Looked at a bunch of different Differential and Maniphest comments; they
appeared to render identically to how they looked before.
- Tested some edge cases like anchors and "show details" on description edits
in Maniphest.
Reviewers: btrahan, jungejason
Reviewed By: btrahan
CC: aran, epriestley
Maniphest Tasks: T904
Differential Revision: https://secure.phabricator.com/D1686
Summary:
First stab at a batch editor for Maniphest. Basically, you can select a group of
tasks and then import them into the "batch" interface, where you can edit all of
them at once.
High level goal is to make it easier for users in PM/filer/support/QA roles to
deal with large numbers of tasks quickly.
This implementation has a few major limitations:
- The only available actions are "add projects" and "remove projects".
- There is no review / undo / log stuff.
- All the changes are applied in-process, which may not scale terribly well.
However, the immediate need is just around projects and this seemed like a
reasonable place to draw the line for a minimal useful version of the tool.
Test Plan: Used batch editor to add and remove projects from groups of tasks.
Reviewers: btrahan, yairlivne
Reviewed By: btrahan
CC: aran, epriestley, sandra
Maniphest Tasks: T441
Differential Revision: https://secure.phabricator.com/D1680
Summary:
This is so freaking cool that I will try to implement it also on Facebook.
Idea is from
http://strd6.com/2011/09/html5-javascript-pasting-image-data-in-chrome/.
I don't know how to properly detect support but lying about it is not a big
deal.
Test Plan:
Go to revision comment textarea.
Paste some text data - works as usual.
Paste some image data in Chrome - file is uploaded and a link to it is inserted.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, epriestley
Differential Revision: https://secure.phabricator.com/D1681
Summary:
I typed up like 30 pages here and then got my soul crushed by T895, but here's
the summary:
I looked at several charting libraries. There aren't very many that seem to be
any good and have an open-source license.
I also want the charts to be scriptable in JS so we can add good interactivity
where appropriate.
Raphael is an SVG drawing library which seems very solid. gRaphael is a charting
library on top of Raphael that is a lot less solid, but seems kind of OK.
Overall, I think this selection gives us a lot of flexibility, although we'll
have to pay some costs up front. I'd rather do that then get limited later,
though.
That said, I'm open to other suggestions here if anyone has experience or wants
to take a different stab at researching things.
This is largely for @vii and D1643.
Test Plan: Created a basic, fairly OK chart (see next revision).
Reviewers: btrahan, vii
Reviewed By: btrahan
CC: aran, epriestley
Differential Revision: https://secure.phabricator.com/D1654
Summary: Update Phabricator for Remarkup changes in D1638.
Test Plan: Looked at various sorts of nested, enumerated, and phantom-item
lists.
Reviewers: btrahan, jungejason
Reviewed By: jungejason
CC: aran, epriestley
Maniphest Tasks: T885
Differential Revision: https://secure.phabricator.com/D1639
Summary:
This is pretty simple and unpolished, but it's getting pretty big and it seems
like a reasonable starting point.
- Log chat in various "channels".
- Conduit record and query methods.
- IRCBot integration for IRC logging
Major TODO:
- Web UI is really unpolished and has no search, paging, anchor-linking, etc.
Basically all presentation stuff, though.
- I think the bot should have a map of channels to log with channel aliases?
- The "channels" should probably be in a separate table.
- The "authors" should probably be correlated to Phabricator accounts somehow,
where possible.
Test Plan: Used phabotlocal to log #phabricator.
Reviewers: kdeggelman, btrahan, Koolvin
Reviewed By: btrahan
CC: aran, epriestley
Maniphest Tasks: T837
Differential Revision: https://secure.phabricator.com/D1625
Summary:
- Restore quick methods for getting to common features (upload file, create
task, etc.)
- Provide a flexible cli-like navigation element similar to stuff used at
Facebook (bunny1 / lolbunny).
Test Plan: Used jump nav and nav buttons.
Reviewers: btrahan, fratrik
Reviewed By: btrahan
CC: aran, epriestley
Differential Revision: https://secure.phabricator.com/D1619
Summary:
Pretty straightforward; see title. Kind of gross but I have a bunch
more iterations in mind here (like filtering). Paging this is a little tricky
since we can't easily use AphrontPagerView, as it relies on OFFSET, and I think
that's sort of sketchy to use here for UX reasons (query performance and view
consistency as feed updates).
Test Plan: Looked at feed, paged through feed.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran, epriestley
Differential Revision: https://secure.phabricator.com/D1616
required
Summary: Make these things like 1/4th the size if they aren't actionable.
Test Plan: Loaded home page with actionable, unactionable panels.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran, epriestley
Differential Revision: https://secure.phabricator.com/D1613
Summary:
'this._request' was never set so 'waiting' was always false.
Result was that several requests were sent at once which wastes resources and
leads to weird bugs when responses don't arrive in sending order.
Blame Rev: D258
Test Plan:
Write a comment extremely fast, watch for requests sent.
Add sleep(5) for some inputs to DifferentialCommentPreviewController, verify
correct order.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, epriestley
Differential Revision: https://secure.phabricator.com/D1612
Summary: It makes perfect sense to add more reviewers while requesting review.
Test Plan:
Request review. Verify that Add Reviewers field shows and works.
Add some reviewer. Verify that comment preview works.
Submit. Verify that reviewers are saved and displayed.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, epriestley
Differential Revision: https://secure.phabricator.com/D1473
Summary:
Some browsers will still sniff content types even with "Content-Type" and
"X-Content-Type-Options: nosniff". Encode "<" and ">" to prevent them from
sniffing the content as HTML.
See T865.
Also unified some of the code on this pathway.
Test Plan: Verified Opera no longer sniffs the Conduit response into HTML for
the test case in T865. Unit tests pass.
Reviewers: cbg, btrahan
Reviewed By: cbg
CC: aran, epriestley
Maniphest Tasks: T139, T865
Differential Revision: https://secure.phabricator.com/D1606
Summary: Looping on this interface is pretty useful but you don't always want to
keep the projects/owners.
Test Plan: Clicked both buttons.
Reviewers: btrahan
Reviewed By: btrahan
CC: anjali, aran, epriestley
Differential Revision: https://secure.phabricator.com/D1593
Summary:
Rough cut that still needs a lot of polish, but replace the directory list with
more of a dashboard type thing:
- Show "Unbreak Now", triage-in-your-projects, and other stuff that you're
supposed to deal with, then feed.
- Move tools a click a way behind nav -- this also lets us put more stuff
there and subtools, etc., later.
- Remove tabs.
- Merge the category/item editing views.
- I also added a light blue wash to the side nav, not sure if I like that or
not.
Test Plan:
- Viewed all elements in empty and nonempty states.
- Viewed applications, edited items/categories.
Reviewers: btrahan, aran
Reviewed By: btrahan
CC: aran, epriestley, davidreuss
Maniphest Tasks: T21, T631
Differential Revision: https://secure.phabricator.com/D1574
Summary:
This addresses a few things:
- Provide a software HTTP response spliting guard as an extra layer of
security, see http://news.php.net/php.internals/57655 and who knows what HPHP/i
does.
- Cleans up webroot/index.php a little bit, I want to get that file under
control eventually.
- Eventually I want to collect bytes in/out metrics and this allows us to do
that easily.
- We may eventually want to write to a socket or do something else like that,
ala Litespawn.
Test Plan:
- Ran unit tests.
- Browsed around, checked headers and HTTP status codes.
Reviewers: btrahan, vrana
Reviewed By: btrahan
CC: aran, epriestley
Differential Revision: https://secure.phabricator.com/D1564
Summary:
This code was just all kinds of wrong, but got all the common cases anyone cares
about correct.
- In edit-inline-comments.js, if isOnRight() is true, use data.right, not
data.left (derp).
- Set data.left correctly, not to the same value as data.right (derp derp).
- Set "isNewFile" based on $is_new, not $on_right (derp derp derp).
Test Plan:
- Added JS debugging code to print "OLD" vs "NEW" and "LEFT" vs "RIGHT".
Clicked the left and right sides of diff-vs-base and diff-vs-diff diffs,
verified output was accurate in all cases.
- Added comments to the left-display-side of a diff-of-diffs, saved them, they
showed up where I put them.
Reviewers: btrahan, vrana
Reviewed By: btrahan
CC: aran, epriestley
Maniphest Tasks: T543
Differential Revision: https://secure.phabricator.com/D1567
Summary:
- Default "personal" vs "global" choice to "personal".
- Don't show global rules under "My Rules".
- After editing or creating a global rule, redirect back to global rule list.
- Use radio buttons for "personal" vs "global" and add captions explaining the
difference.
- For "global" rules, don't show the owner/author in the rule detail view --
they effectively have no owner (see also D1387).
- For "global" rules, don't show the owner/author in the rule list view, as
above.
- For admin views, show rule type (global vs personal).
Test Plan:
- Created and edited new global and personal rules.
- Viewed "my", "global" and "admin" views.
Reviewers: btrahan, jungejason, nh, xela
Reviewed By: btrahan
CC: aran, epriestley
Differential Revision: https://secure.phabricator.com/D1518
Summary:
Render coverage information in the right gutter, if available.
We could render some kind of summary report deal too but this seems like a good
start.
Test Plan:
- Looked at diffs with coverage.
- Looked at diffs without coverage.
- Used inline comments, diff-of-diff, "show more", "show entire file", "show
generated file", "undo". Nothing seemed disrupted by the addition of a 5th
column.
Reviewers: btrahan, tuomaspelkonen, jungejason
Reviewed By: btrahan
CC: zeeg, aran, epriestley
Maniphest Tasks: T140
Differential Revision: https://secure.phabricator.com/D1527
Summary:
This diff restructures the DOM and alters some CSS within differential.
Original goal was to unify these codepaths more fully into a base class or
classes, but they have quite a bit of custom code such that didn't feel too
compelling in practice. It also felt related to feed stories as I thought
about the more general version(s) of this code...
Also deleted some CSS from maniphest that wasn't doing anything.
Test Plan:
looked at a differential diff and liked what I saw. spent a bunch
of time trying out different types of comments and etc.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, epriestley
Maniphest Tasks: T803
Differential Revision: https://secure.phabricator.com/D1513
task metadata panel.
Summary: see title.
Test Plan: it looks good! also increased and decreased the font-size and
verified the width remained consistent. did this on chrome and firefox on a
mac.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, epriestley
Differential Revision: https://secure.phabricator.com/D1494
Summary: Make it easy to join or leave (well, slightly less easy) a project.
Publish join/leave to feed. Fix a couple of membership editor bugs.
Test Plan: Joined, left a project.
Reviewers: btrahan, jungejason
Reviewed By: btrahan
CC: aran, epriestley
Maniphest Tasks: T681
Differential Revision: https://secure.phabricator.com/D1485
Summary: also fixes a small bug where the page title was always "Create Task".
switch it to the header name which is much more descriptive / correct IMO.
Test Plan:
created a new task and watched the description preview update.
edited an old task and saw the description preview populate with the correct
existing data.
edited an old task and edited the description and saw the description preview
update
Reviewers: epriestley
CC: aran, epriestley
Differential Revision: https://secure.phabricator.com/D1489
Summary:
It is possible to open a file in editor by registering a custom URI scheme
(pseudo-protocol). Some editors register it by default.
Having links to open the file in external editor is productivity booster
although it is a little bit harder to set up.
There are several other tools using file_link_format configuration directive
(XDebug, Symfony) to bind to this protocol.
I've added the example with editor: protocol which can be used as a proxy to
actual editor (used by Nette Framework:
http://wiki.nette.org/en/howto-editor-link).
Test Plan:
Configure Editor Link in User Preferences.
Register URI scheme in OS.
Open a file in Diffusion. Click on the Edit button.
Open a revision in Differential. Click on the Edit button.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, epriestley
Differential Revision: https://secure.phabricator.com/D1422
Summary:
Added a typeahead in the edit herald rule page that allows an admin or
owner to change the current owner of a rule. If the typeahead is emptied, the
current owner will remain owner.
Test Plan:
Created a test rule. Changed the owner. Deleted the owner in the
typahead. Verified expected behavior.
Reviewers: jungejason, epriestley
Reviewed By: epriestley
CC: aran, jungejason, epriestley, xela
Differential Revision: https://secure.phabricator.com/D1322
Summary: The <a href> attribute is useful because user knows where the link goes
before opening it plus he can copy it to the clipboard plus he can add it to the
bookmarks.
Test Plan:
Display revision.
View Options.
Click.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran
Differential Revision: https://secure.phabricator.com/D1436
Summary:
We currently allow you to assign code review to disabled users, but
should not.
Test Plan:
- Created revisions with no reviewers and only disabled reviewers, was
appropriately warned.
- Looked at a disabled user handle link, was clearly informed.
- Tried to create a new revision with a disabled reviewer, was rebuffed.
Reviewers: btrahan, jungejason
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D1429
Summary:
See D1416. Add options to file-embed syntax, and document new code and
embed options.
Test Plan: Used new options in markup blocks.
Reviewers: davidreuss, btrahan, jungejason
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T336
Differential Revision: https://secure.phabricator.com/D1417
Test Plan:
Open menu for added file
Open menu for deleted file
Open menu for changed file
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, epriestley
Differential Revision: https://secure.phabricator.com/D1410
Summary:
With T764, http://localhost doesn't work anymore. So add instructions
about how to support it by modifying the hosts file.
Test Plan:
- turned on setup mode and the error message did show up
- turned off the setup mode and the error message also showed up
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, epriestley
Maniphest Tasks: T764
Differential Revision: https://secure.phabricator.com/D1370
Summary:
Chrome/Chromium won't set cookies on these domains, at least under
Ubuntu. See T754. Detect brokenness and explode.
Test Plan:
Logged into phabricator as "http://derps/" (failed) and
"http://derps.com/" (worked) in Chromium. Set config to "http://derps/" (config
exploded) and "http://local.aphront.com/" (config OK).
Reviewers: btrahan, jungejason
Reviewed By: btrahan
CC: aran, btrahan
Maniphest Tasks: T754
Differential Revision: https://secure.phabricator.com/D1355
Summary: Not really thrilled about my fix for T684 in D1224. This makes some
design tweaks to solve it without the awkward horizontal scrollbar in the page
content div.
Test Plan: Looked at diffs overflowing the window. Looked at footer on several
pages.
Reviewers: btrahan, jungejason, Makinde
Reviewed By: btrahan
CC: aran, btrahan
Maniphest Tasks: T684
Differential Revision: https://secure.phabricator.com/D1332
Summary: Clicks all the "Show All" links for you at the touch of a button.
Test Plan:
- Used "reveal entire file" on revealable files.
- Opened on already-visible files, got "entire file shown".
- Used other menu options.
- Used normal "show more" links.
Reviewers: btrahan, jungejason
Reviewed By: btrahan
CC: aran, epriestley, btrahan
Maniphest Tasks: T497
Differential Revision: https://secure.phabricator.com/D1331
Summary:
We currently don't link to comments which aren't visible. Link to the
appropriate diff in a new window, indicating where the comment lives.
Test Plan: Clicked visible, not-so-visible comments.
Reviewers: btrahan, jungejason, davidreuss
Reviewed By: btrahan
CC: aran, btrahan, epriestley
Maniphest Tasks: T555, T449
Differential Revision: https://secure.phabricator.com/D1333
Summary:
Provide an easy way to jump to Diffusion from Differential if we have
the data we need to connect them.
Test Plan: Tested menu in linked and unlinked diffs. Used menu item.
Reviewers: btrahan, jungejason
Reviewed By: btrahan
CC: aran, epriestley, btrahan
Maniphest Tasks: T309
Differential Revision: https://secure.phabricator.com/D1326
Summary:
When the user loads a page with an anchor on it like #thing, or clicks a link to
#thing, and #thing doesn't exist, keep trying to navigate to #thing for a few
seconds.
This allows anchors to work when the target is in content which is later ajaxed
in. In particular, this affects inline comments in Differential.
Test Plan: Opened inline comment links in a new tab, was in the right place when
I switched tabs.
Reviewers: nh, btrahan, jungejason
Reviewed By: btrahan
CC: aran, btrahan, epriestley
Maniphest Tasks: T492
Differential Revision: https://secure.phabricator.com/D1327
Summary:
There are several open Differential tasks that are basically blocked on not
having reasonable places in the UI to put things. Replace the "View Standalone /
Raw" button with a "View Options" dropdown menu so we can shove things like
"Expand All", "Fold / Unfold File", and "View in Diffusion" in there.
This doesn't change any behavior, just puts the existing options in a menu.
Test Plan:
- Toggled menu open by clicking button.
- Clicked menu items.
- Toggled menu closed by clicking button.
- Toggled menu closed by clicking document.
- Toggled menu closed by opening another menu.
- Toggled menu closed by selecting an item.
Reviewers: btrahan, jungejason
Reviewed By: btrahan
CC: aran, btrahan
Maniphest Tasks: T497, T309
Differential Revision: https://secure.phabricator.com/D1316
Summary:
- Use n/p to jump between comments.
- Use r to reply to the selected comment.
- Use e to edit the selected comment.
Test Plan: Verified n, p, r, e, j, k, J, K, "click edit" and "click reply"
behavior in as many weird cases as I could come up with.
Reviewers: btrahan, jungejason, nh, cpiro, jl
Reviewed By: btrahan
CC: aran, btrahan, epriestley
Maniphest Tasks: T583
Differential Revision: https://secure.phabricator.com/D1308
Summary: Preview of Add Reviewers looks silly without actually showing them
Test Plan:
Go to any diff
Leap into action: Add Reviewers
Add some reviewers
Write some comment
Preview including Added reviewers should be displayed
Change action to Comment
Added reviewers should disappear
Repeat with Add CCs
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, epriestley, vrana
Differential Revision: https://secure.phabricator.com/D1276
Summary:
Create a visual hierarchy with the <span>s and <a>s in the aphront-side-nav
so people don't try to click on a span thinking it's a link. This is to
help specifically with the case of the "All Revisions" header on the
differential revision list page - I've had a few people ask about that
broken link.
Test Plan:
Loaded the differential revision list view and the maniphest task list
view to check that their left-hand navs look ok (did this in ff8 on ubuntu
11.10).
Reviewers: epriestley, jungejason, btrahan
Reviewed By: jungejason
CC: aran, jungejason, epriestley
Differential Revision: https://secure.phabricator.com/D1303
Summary:
If a page generates warnings or errors, you only get a little red dot in
DarkConsole which is hard to see. DarkConsole is also fairly big and there are
plenty of reasons not to leave it open all the time.
Instead, unconditionally show a big message to developers if there are errors or
warnings.
We could make this more sophisticated eventually, but the value is just that you
see it.
Test Plan: Browsed pages with and without warnings, got the right banner state.
Reviewers: nh, btrahan, jungejason
Reviewed By: btrahan
CC: aran, btrahan
Maniphest Tasks: T734
Differential Revision: https://secure.phabricator.com/D1307
Summary: Rate-limit conditions didn't set a new timer. It results in stopping of
periodically updating Preview and also in missing last typed characters in
Preview.
Test Plan:
Go to any diff
Type something really fast in Comment
After finishing typing, whole comment should be displayed in Preview
Insert something without keyboard (e.g. paste with mouse)
Preview should be updated
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, epriestley
Differential Revision: https://secure.phabricator.com/D1288
Summary:
- On the edit view, this is represented as a checkbox.
- On the detail view, it renders with a user-selectable string.
Test Plan: Added a bool field to my local install, checked and unchecked it.
Reviewers: zeeg, jungejason, btrahan
Reviewed By: jungejason
CC: aran, jungejason
Differential Revision: https://secure.phabricator.com/D1277
Summary:
- Old page was useless and dumb.
- New page looks a little less bad, functions a little less poorly.
- Still lots of work to be done.
Test Plan:
- Viewed a project.
- Clicked all the links on the left nav.
- Here is a screenshot:
https://secure.phabricator.com/file/view/PHID-FILE-4buzquotb3fo4dhlicrw/
Reviewers: btrahan, jungejason
Reviewed By: jungejason
CC: aran, jungejason
Maniphest Tasks: T681
Differential Revision: 1246
Summary:
This seems like the least-bad solution to the issues mentioned in T684: when we
need to x-scroll the main page area, scroll that div rather than the surrounding
page chrome.
I played around with a bunch of other possible solutions but they all seem bad
in some way or another. The tricky part here is that I want the real background
to be grey so that the footer color is grey even if the page is very short and
the browser window is very tall.
The only downside here is that the scrollbar appears in a somewhat unusual
place, but I think that's OK?
Actually, it's kind of terrible if people really use the scrollbar to scroll
horizontally rather than two-finger swipe or shift+mousewheel or the arrow keys.
So maybe this isn't good.
If this is no good, I think we need to make design sacrifices (not necessarily a
big deal; I'm not married to how the footer behaves) or someone much better than
I am at CSS needs to tell me how to fix this (@mroch / @tomo)?.
Test Plan:
- In Settings -> Preferences, set font to "72px Impact".
- Observed overflow scroll behavior in Safari / Firefox / Chrome.
Reviewers: Makinde, btrahan, jungejason
Reviewed By: Makinde
CC: mroch, tomo, aran, Makinde
Maniphest Tasks: T684
Differential Revision: 1224
