Summary:
- Feature request from Airtime that I missed in the feedback notes, came up yesterday.
- Identify git submodules as "FILE_SUBMODULE", not "FILE_NORMAL".
- Link git submodules to an external resolver endpoint, which tries to find commits in tracked repositories.
- Identify git symlinks as "FILE_SYMLINK", not "FILE_NORMAL".
- Add folder, file, symlink and externals icons.
Test Plan:
- externals/javelin is now identified as a submoudule and links to Javelin, not identified as a file and links to error.
- bin/phd is now identified as a symlink.
- Interfaces have pretty icons.
Reviewers: btrahan, cpiro, ddfisher, keebuhm, allenjohnashton
Reviewed By: btrahan
CC: aran, epriestley
Differential Revision: https://secure.phabricator.com/D1975
Summary:
- Affects the "Inline Comments" summary table which appears in comments that have attached inlines in the discussion threads in Differential.
- Prepares for inclusion in Diffusion.
- No application changes (minor CSS), just factors code better.
- Simplify/separate CSS.
Test Plan: Looked at on-diff and off-diff comment summaries in Differential, display looked correct.
Reviewers: davidreuss, nh, btrahan
Reviewed By: davidreuss
CC: aran, epriestley
Maniphest Tasks: T904
Differential Revision: https://secure.phabricator.com/D1928
Summary:
Make clicking the link also select the object (this operation is very common). Add an arrow to the left to view the object (this operation is very rare). Increase link target area to the entire cell.
Also simplify some handlers.
Test Plan: Clicked things with wild abandon. Behavior seemed unchanged.
Reviewers: cpiro, btrahan
Reviewed By: cpiro
CC: aran, epriestley
Differential Revision: https://secure.phabricator.com/D1962
Summary:
It looks like there is really this text written e.g. at https://secure.phabricator.com/D1896#0a6a1957
I am not sure that it is the only place which needs to be fixed.
Test Plan: Display diff with no newline at end of file in Differential.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, epriestley
Differential Revision: https://secure.phabricator.com/D1900
Summary:
- Show a tip in the margin about what coverage colors mean.
- Highlight the line when mousing over coverage.
- Randomly change the colors to different colors.
- Fix a bug with "show more" that I introduced with the other coverage diff (oops!)
Test Plan: Moused over coverage things.
Reviewers: tuomaspelkonen, btrahan
Reviewed By: tuomaspelkonen
CC: aran, epriestley
Maniphest Tasks: T965
Differential Revision: https://secure.phabricator.com/D1874
Summary:
- Remove "0.5%" padding which makes Safari flip out and render every row differently sometimes.
- Remove list padding from ManiphestTaskListView, put it in the controller composition instead.
Test Plan: Viewed all places where task lists appear.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran, epriestley
Differential Revision: https://secure.phabricator.com/D1788
Summary:
- Update the Javelin submodule to pick up recent fixes (like D1749).
- Update the package definitions do do a slightly better job of packaging
resources.
Test Plan:
Up and down work in tokenizers now. Pages load slightly fewer
resources.
Reviewers: vrana, btrahan
Reviewed By: btrahan
CC: aran, epriestley
Maniphest Tasks: T927
Differential Revision: https://secure.phabricator.com/D1751
Summary:
- These are still slow, awkward and hideous -- but slightly better than
before.
- Allow "open" reports to be sorted.
- Add a "burn" chart/table for assessing project volatility.
- Add navigation.
Test Plan: Looked at reports.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran, epriestley
Maniphest Tasks: T923
Differential Revision: https://secure.phabricator.com/D1737
Summary:
Some text editors support opening multiple files at once.
I've used space as paths separator which may be compatible with some other
editors (I didn't tried any other though).
Note: This approach is incompatible with spaces in paths.
I am fine with changing it to anything else to support such paths or more
editors.
Probably the cleanest solution (yet still incompatible with most editors) would
be to use something like ##editor://open/?file=A&line=1&file=B&line=2## but it
would require also changing the way how it's configured and I think it's not
worth it.
BTW, I've used a hacky bookmarklet for this feature before.
Deleted or added paths may not exist in users filesystem but we don't know which
so the button tries to open everything.
Test Plan:
Click Edit All.
Delete Editor Link in settings, verify that the button is missing.
View diff without revision, verify that the button is missing.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, epriestley
Differential Revision: https://secure.phabricator.com/D1741
Summary:
The current approach of using a modal overlay dialog to create/edit inline
comments is pretty silly. Use an inline textarea instead.
This element isn't perfect and we have some mild modalness issues, but I think
it's better than the silly thing we've got going on right now. We can keep
poking it as people break it.
Test Plan:
- Created comments; submitted and undid them in empty and nonempty states.
Used undo for nonempty states + cancel.
- Edited comments; saved and canceled them. Used undo for changed state.
- Replied to comments; yada yada as above.
- Deleted comments.
- Did various modal trickery where I clicked "Reply" on something else with a
dialog already up, this very mildly glitches but I think it's not a big issue.
Reviewers: vrana, btrahan, Makinde, nh
Reviewed By: vrana
CC: aran, epriestley
Maniphest Tasks: T431
Differential Revision: https://secure.phabricator.com/D1716
Summary:
The general idea here is to build a Differential-like dashboard which shows all
the things you need to audit and all the things that other people have raised
issues with, so you have a one-stop "what do I need to deal with?" interface.
- Add problem commits to the "active" view of /audit/.
- Add problem commits to homepage.
- Add commit browsing interfaces to /audit/.
- Add an "Audit" app button.
Test Plan: Looked at homepage, commit filters. Audited commits, verified state
changes reflected properly.
Reviewers: btrahan, jungejason
Reviewed By: btrahan
CC: aran, epriestley
Maniphest Tasks: T904
Differential Revision: https://secure.phabricator.com/D1712
Summary:
- Move the buttons in the jump nav to iOS-style "application" buttons in the
header. These are sort of ugly right now, but I think serviceable enough. Some
day we will hire a designer whose entire job is to pick up after me.
- This gives us more room (allowing us to restore "Maniphest" and
"Differential").
- This also disassociates the app buttons from the jump nav, which was a
point of confusion (user expectation that the text input is related to the
buttons).
- Allow "Active Revisions" and "Assigned Tasks" to collapse completely. They
didn't completely collapse before because the top-level "Active Tasks" / "Active
Revisions" was sort of overloaded as quick nav to apps. Now we have app buttons.
- Reduce overall size of jump nav.
Test Plan: Looked at homepage in various states of need-for-attention.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran, epriestley
Differential Revision: https://secure.phabricator.com/D1694
Summary:
Current approach has several problems:
- if there is no link in the cell then it still shows a link cursor
- if there is a link then it is clickable only on the text
Test Plan:
Display file in Differential, hover over cell with link.
Repeat for Paste.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, epriestley
Differential Revision: https://secure.phabricator.com/D1701
Summary: We already allow you to create comments, but we don't show them on the
commit page. After style / view unification this is easy; show comments on the
commit page.
Test Plan: Made comments on a commit using the audit too, saw them show up in
Diffusion.
Reviewers: btrahan, jungejason
Reviewed By: btrahan
CC: aran, epriestley
Maniphest Tasks: T904
Differential Revision: https://secure.phabricator.com/D1687
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 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
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:
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:
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:
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
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:
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:
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:
- 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
Summary:
...except that pesky help tab which remains.
Pertinent bits here...
- move "History" into button "View History" that is grey and next to "Edit Page"
- for history page, add breadcrumb similar to the one on "diff" page. This
unifies the experiencing on history <=> diffs as well as gives the user a link
back to the document, which was a tab on the History page before this diff.
Thoughts for next time...
- I'd like to further unify the breadcrumbs between "View" and "History / Diff".
- The "Document Index" is pretty sweet and feels a bit buried. I wonder if
unifying breadcrumbs is the key here?
Test Plan: clicked around phriction. viewed a document, viewed its history.
verified links in breadcrumbs were correct
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, epriestley
Maniphest Tasks: T631
Differential Revision: 1221
Summary:
Because D1028 caused the column containing differential revision property
labels to have a fixed width, some custom labels are longer than what fits
(and it makes more sense to word wrap them instead of making the column
wider).
I also updated the corresponding maniphest css for consistency.
Test Plan:
Used firebug to remove css property and visually check that is the intended
effect; loaded a page after the revision and saw that the css property is
no longer set, allowing the labels to wrap.
Reviewers: epriestley, jungejason
Reviewed By: jungejason
CC: aran, jungejason, epriestley
Differential Revision: 1066
Summary: See T551. We don't apply the default monospacing rules to the example,
so if you don't have a custom font selection you don't see the default
accurately.
Test Plan: Deleted my preference, saw an accurate default. Set my preference to
"14px impact", ensured it was respected in applications.
Reviewers: jungejason, nh, tuomaspelkonen, aran
Reviewed By: nh
CC: aran, nh
Differential Revision: 1035
Summary:
Sometimes, elements in a property table at the top of a differential
revision view or maniphest task detail view will have a minimum width
that is too wide to fit in the table without causing the table's width
to exceed the width of its parent div. This diff changes the table layout
algorithm so that the table's width never exceeds the width of its parent
div. In the case of a code block causing the excess width, it puts a
scrollbar on the block instead of letting content spill out.
Due to the way the fixed table layout algorithm works, the width of the
left column (containing headers) is set to a fixed width. I chose a width
for differential that works with the default headers, but site-specific
headers might not fit.
Test Plan:
Created a task, added a code block in the description that had an
unreasonably long line in it, and visually verified that the <td>
containing the <code> did not expand horizontally past the limit defined
by the <div> containing the <table>. I also loaded a differential revision
view and checked that its table looks sane.
Reviewers: epriestley, jungejason, aran
Reviewed By: epriestley
CC: aran, nh, epriestley
Differential Revision: 1028
Summary:
The differential panels at the top of the differential revision view page
were 2px smaller than the divs on the bottom of the page (everything below
the table of contents). This diff makes differential-panel 2px wider so it
matches.
Test Plan: viewed a differential revision and checked that the divs lined up
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, epriestley
Differential Revision: 887
Summary:
oh god everyone hates this
revert revert
https://www.facebook.com/photo.php?fbid=787360256660&set=p.787360256660&type=1&theater
(I left the icons themselves since I have some plans to do other things with
them.)
Test Plan: I am not good at designer
Reviewers: ola, elynde, bh, ashwin, jungejason, kdelong, zrait, tomo, aran
Reviewed By: aran
CC: aran, epriestley, tomo
Differential Revision: 885
Summary:
After D857, we try to attach local commit information to revisions. If this
information is available, display it on the revision.
Design on this is a little rough, I might try to combine this into the revision
update view or something like that since we're starting to take up a lot of real
estate for metadata.
Test Plan: Local diffed this and got some commit info.
Reviewers: jungejason, tuomaspelkonen, aran
Reviewed By: jungejason
CC: aran, epriestley, jungejason
Differential Revision: 872
