1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-11-30 10:42:41 +01:00
Commit graph

3030 commits

Author SHA1 Message Date
epriestley
83e99fb691 Add a class to the Differential banner when unsubmitted/unsaved changes are present
Summary: Ref T12733. This adds classes for unsubmitted/unsaved changes, and lays some groundwork for additional buttons.

Test Plan:
  - Added, edited, deleted comments.
  - Saw bar background color update appropriately.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12733

Differential Revision: https://secure.phabricator.com/D18045
2017-05-30 17:59:23 -07:00
epriestley
cc0a6fd3aa Remove the ability to leave multi-line inline comments on touchscreen devices
Summary: Ref T12733. This ultimately conflicts with scrolling and took about two days to get reported as a bug/regression. See T12733 for a bunch of additional discussion. See T1026 for original discussion.

Test Plan:
  - Left single-line and multi-line comments on desktop.
  - Tapped to leave single-line comments on mobile.
  - Dragged lines on mobile, got a scroll instead of a range comment.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12733

Differential Revision: https://secure.phabricator.com/D18044
2017-05-30 17:59:07 -07:00
epriestley
d20221dc7d Remove Differential "objectives" UI
Summary: Ref T12733. Completely removes the objectives UI.

Test Plan:
  - Grepped for `objective`, etc.
  - Browsed revisions, no JS errors / broken stuff.
  - (If I missed anything, it's likely to turn up in followup changes.)

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12733

Differential Revision: https://secure.phabricator.com/D18043
2017-05-30 17:58:49 -07:00
Chad Little
c5bb69fd7d Use a list view for DiffusionHistory
Summary: This moves Diffusion History to use an easier to parse list view for commits and their (diff, audit, build) status. I left TableView around, which is used on a repositories home, and we can maybe add a "graph view" history back as another controller. Not sure what the real use is for that kind of feature though. I don't have Harbormaster set up locally so I could use another install to give this a run. I also expect to maybe not live with this UI as final, I like the UX, but the icons for indicating status don't really feel great to me, just OK.

Test Plan:
pull various repositories, check various history displays.

{F4980356}

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D18039
2017-05-30 17:31:48 -07:00
Chad Little
e5b3d03319 Fix lightbox circle icons
Summary: These are unfortunatly manually built so I missed them in testing circle view changes.

Test Plan: Test lightbox, conpherence, uiexamples

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D18036
2017-05-26 20:12:56 -07:00
Austin McKinley
04fd93e51e Drop DifferentialDraft storage
Summary: Fixes T12104.

Test Plan: Ran `bin/storage upgrade` and observed table dun got dropped.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: epriestley

Maniphest Tasks: T12104

Differential Revision: https://secure.phabricator.com/D18034
2017-05-26 13:59:26 -07:00
Chad Little
81809713e0 Try layering state icons on PHUICircleIconView
Summary: I think this is reasonable for my current use case, but stacking icons overally is pretty clunky.

Test Plan:
UIExamples

{F4978899}

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D18032
2017-05-26 13:55:42 -07:00
Austin McKinley
b27c2ed6d1 Index Project milestones to accurately reflect milestone membership
Summary: Fixes T12505. `PhabricatorProjectsMembershipIndexEngineExtension->materializeProject()` was incorrectly bailing early for milestone objects, which prevented milestone members from being calculated correctly. This was causing problems where (for example) an Owners package owned by a milestone wasn't being satisfied when a member of the milestone approved a revision.

Test Plan: Invoked migration, observed that a user's milestones correctly showed up when searched for. Also observed that accepting a revision on behalf of a milestone now satisfies Owners rules.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T12505

Differential Revision: https://secure.phabricator.com/D18033
2017-05-26 13:10:41 -07:00
Chad Little
718c39131d Add a little style to Phriction ToC menu
Summary: Adds some indentation and color. Ref T9868.

Test Plan: A long page with multiple indentation levels.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T9868

Differential Revision: https://secure.phabricator.com/D18025
2017-05-26 09:17:30 -07:00
Chad Little
9bbea869b3 Move setLaunchButton to setSideColumn for ObjectItem
Summary: Makes this a bit more flexible and allow UI to take over `col-2` completely. Also cleaned up application search a little with tags

Test Plan: Review various pages, grep for callsites.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D18021
2017-05-25 15:31:19 -07:00
Chad Little
6b3d04683d Clean up SIMPLE button styles
Summary: Some of these are unused, defaults to a lighter color naturally.

Test Plan: uiexamples, grep, phriction

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D18020
2017-05-25 14:53:59 -07:00
Austin McKinley
f29b54944f Make closed objects in global typeahead look closed
Summary:
Fixes T6906.

I found the code in `behavior-search-typeahead.js` that was throwing away the closedness-detction work done in `Prejab.js::transformDatasourceResults`. Modified it to re-add the correct class name to the `phabricator-main-search-typeahead-result` elements.

Then I found some CSS in `typeahead-browse.css` and completely flailed around until realizing that particular CSS only gets loaded when hitting the typeahead endpoint directly. Copied the relevant bit of CSS over to `main-menu-view.css` (but maybe it should be removed from `typeahead-browse.css`?).

This is my first JS/CSS change, so please don't assume I did anything right.

Test Plan: {F4975800}

Reviewers: #blessed_reviewers, chad

Reviewed By: #blessed_reviewers, chad

Subscribers: epriestley

Maniphest Tasks: T6906

Differential Revision: https://secure.phabricator.com/D18017
2017-05-25 13:43:53 -07:00
epriestley
7e91f42b02 Issue commands to Nuance items, at least roughly
Summary:
Ref T12738. This makes clicking "Throw In Trash" technically do something, sort of.

In Nuance, the default mode of operation for actions is asynchronous -- so you don't have to wait for a response from Twitter or GitHub after you mash the "send default reply tweet" / "close this pull request with a nice response" button and can move directly to the next item instead.

In the future, some operations will attempt to apply synchronously (e.g., local actions like "ignore this item forever"). This fakes our way through that for now.

There's also no connection to the action actually doing anything yet, but I'll probably rig that up next.

Test Plan: {F4975227}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12738

Differential Revision: https://secure.phabricator.com/D18010
2017-05-24 11:04:57 -07:00
epriestley
272a5d668f Fix a handful of Nuance fatals
Summary: Ref T12738. Some of the Nuance "form" workflows currently fatal after work on the GitHub stuff. Try to make everything stop fataling, at least.

Test Plan: Using "Complaints Form" no longer fatals, and now lodges a complaint instead.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12738

Differential Revision: https://secure.phabricator.com/D18007
2017-05-24 11:02:55 -07:00
Chad Little
684ce701fb Add a description/toggle to PHUIObjectItemView
Summary: Gives the ability to hide a big long block of text in an ObjectListItem without cluttering the UI.

Test Plan:
Added a test case to UIExamples. Click on icon, see content. Click again, content go away.

{F4974153}

{F4974311}

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D18006
2017-05-24 09:18:13 -07:00
Chad Little
1b36252ef3 Add a dedicated HistoryListView for Diffusion
Summary: Going to play a bit with this layout (diffusion sans audit) and see how it feels on profile. Uses a user image, moves the commit hash (easily selectible) and separates commits by date.

Test Plan:
Review profiles with and without commits.

{F4973987}

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D18005
2017-05-23 14:03:49 -07:00
Chad Little
03d4d674f8 Clean up some colors missing from PHUITagView type shade
Summary: Grep for phui-tag-shade and verify we're no longer calling shade-color directly.

Test Plan: Search, workboard, story points, etc.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D17993
2017-05-22 10:52:10 -07:00
Chad Little
5d966897f1 Add Outline tag type to PHUITagView
Summary: Adds a new tag type, starts to try to clean up the mess that are PHUITags

Test Plan:
Review UIExamples.

{F4972323}

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D17991
2017-05-22 10:16:27 -07:00
epriestley
7d44e7cb4d Show the curent selected inline in the objective list
Summary: Ref T12733. When an inline is selected, make it stand out so you can see where you are in the document more clearly.

Test Plan: {F4968509}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12733

Differential Revision: https://secure.phabricator.com/D17982
2017-05-20 08:01:21 -07:00
epriestley
e15009b76e Show "reply" inlines as replies in the objective list
Summary: Ref T12733. Show a "reply" icon for replies, and make them stack directly under their parent.

Test Plan: {F4968500}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12733

Differential Revision: https://secure.phabricator.com/D17981
2017-05-20 08:00:50 -07:00
epriestley
4dff754502 Show a snippet when hovering inlines in the objective list
Summary: Ref T12733. Shows a comment snippet when hovering inlines in the objective list.

Test Plan: {F4968490}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12733

Differential Revision: https://secure.phabricator.com/D17980
2017-05-20 08:00:09 -07:00
epriestley
7a40dd380e When a user cancels a new inline, clear it from the objective list
Summary: Ref T12733. Currently, creating a new inline and then canceling it leaves a marker in the objective list. Instead, remove the marker.

Test Plan:
  - Created an empty inline, cancelled. Created a non-empty inline, cancelled. No objective marker in either case.
  - Created a new normal inline, objective marker.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12733

Differential Revision: https://secure.phabricator.com/D17979
2017-05-20 07:59:39 -07:00
epriestley
c056ff56cc Fix a diff objective issue where objectives could appear in the wrong place
Summary:
Ref T12733. When creating a new comment, the objective could appear to far up in the scrollbar because we were anchoring it to an invisible row.

Instead, anchor to the "edit" row while editing.

Test Plan: Created a new comment at the very top of a file, saw "File, Star" icons instead of "Star, File".

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12733

Differential Revision: https://secure.phabricator.com/D17978
2017-05-20 07:59:04 -07:00
epriestley
af07600aaa Make Differential objective markers show a brighter "editing" state
Summary:
Ref T12733.

  - While editing a comment, show a pink star ({icon star, color=pink}) with a tooltip.
  - Slight UI tweaks, including draft comments getting an indigo pencil ({icon pencil, color=indigo}).

Test Plan: {F4968470}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12733

Differential Revision: https://secure.phabricator.com/D17977
2017-05-20 07:57:38 -07:00
epriestley
8b2a06387d Stop long filenames in objective list tooltips from being cut off
Summary: Ref T12733. Currently, long filenames get cut off at 160px. Instead, don't cut them off.

Test Plan:
Before:

{F4968401}

After:

{F4968402}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12733

Differential Revision: https://secure.phabricator.com/D17975
2017-05-20 07:57:08 -07:00
epriestley
aba209e999 Hide the Differential scroll objective list on trackpad systems
Summary:
Ref T12733. In the longer run I'd like to just push this out from the edge, but that currently gets us into trouble since we start bumping into content. On my system, the trackpad scrollbar also expands in size when moused over, so the minimum number of pixels we need to push it out is approximatley 15px. This hits body content and the persistent chat.

For now, just disable this element on trackpad systems.

Test Plan:
Disconnected all USB peripherals, quit and relaunched Safari, saw no objective list.

Reconnected mouse, relaunched Safari, saw objective list.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12733

Differential Revision: https://secure.phabricator.com/D17974
2017-05-20 07:56:21 -07:00
epriestley
bdecff7d67 Show "objectives" UI only if prototypes are enabled
Summary: See D17955.

Test Plan: Loaded a revision, no longer saw annotations with prototypes off. Still saw annotations with prototypes on.

Reviewers: chad

Reviewed By: chad

Differential Revision: https://secure.phabricator.com/D17983
2017-05-20 07:55:48 -07:00
epriestley
6945e80fee For the diff banner, detect the current changeset better
Summary:
Ref T12733. Currently, we detect the changeset which is in the middle of the screen as the current changeset.

This doesn't always get us the most intuitive changeset, particularly after a navigation from the scroll objective list: when you jump to changeset "X", you'd tend to expect "X" to be shown in the header, but the //next// changeset may be shown if "X" is too short.

Instead, select the changeset near the top of the screen (spanning an invisible line slightly below the banner).

Test Plan: Scrolled and jumped through a document with long and short changesets, saw a more intuitive changeset selected by the banner.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12733

Differential Revision: https://secure.phabricator.com/D17976
2017-05-20 07:04:48 -07:00
epriestley
fdf00f6df4 Clean up some minor UI behaviors in Differential
Summary:
Minor UI tweaks:

  - Use the dynamic icon for each file (e.g., image, text), not a hard-coded icon.
  - Render the path (less important) in grey and the filename (more important) in black.

Test Plan: {F4966176}

Reviewers: chad

Reviewed By: chad

Differential Revision: https://secure.phabricator.com/D17957
2017-05-19 12:01:58 -07:00
epriestley
6c46f27d98 Add quest objectives to the minimap
Summary:
Add important objectives (like waygates and quest markers) to the minimap.

This also probably fixes @cspeckmim's bug with the {key @} keyboard shortcut.

Test Plan:
(This is probably easier to undestand if you `arc patch` + click around.)

{F4966037}

Reviewers: chad, amckinley

Reviewed By: chad

Subscribers: cspeckmim

Differential Revision: https://secure.phabricator.com/D17955
2017-05-19 12:01:01 -07:00
Chad Little
93e28da76e Add more "disabled" UI to PHUIObjectItemView
Summary: Brings more UI tweaks to disabled objects, like projects/people. Also fixes a missing icon in projects.

Test Plan: Application search with people and projects that have disabled results.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T12732

Differential Revision: https://secure.phabricator.com/D17962
2017-05-19 17:54:53 +00:00
Austin McKinley
f92059d84c Migrate Project status to modular transactions
Test Plan: Unit tests pass. Archived/activated some projects a couple times; observed expected transactions on timeline.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: epriestley

Differential Revision: https://secure.phabricator.com/D17953
2017-05-18 11:36:13 -07:00
Chad Little
1599c56217 Add Pinboard Items to Timeline
Summary: This allows adding of pinboard items to a timeline. I'm hoping we can get this in for Maniphest (Pholio, Cover Image) and Macro (because, Macro), but unsure how to scalably do this. Anyways, here's the front end.

Test Plan:
Make some fake timeline items in UIExamples, test mobile, tablet, and desktop breakpoints.

{F4965798}

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D17950
2017-05-18 10:34:57 -07:00
epriestley
fb9f3cc0b4 Restore the "buoyant" header in Differential
Summary:
Fixes T1591. This was removed long ago because it was a mess to implement and caused a bunch of weird issues, and also my tolerance for dealing with weird JS issues was much, much lower.

I have now survived the fires of JX.Scrollbar and would love to address 200 small nitpicks about obscure browser behaviors on Linux, so open the floodgates again.

A secondary goal here is to create room to add a global view state menu on the right, with 300 options like "hide all inlines", "hide done inlines", "hide collapsed inlines", "hide ghosts", "show ghosts", "enable filetree", "disable filetree", etc, etc. Not sure how much of this I'll actually do. I have one more experiment I want to try first.

Test Plan: {F4963294}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T1591

Differential Revision: https://secure.phabricator.com/D17945
2017-05-18 10:24:26 -07:00
epriestley
f78ce156f1 Restore "h" to hide or show files, and modernize file visibility toggling
Summary:
Ref T12616. This puts "h" back to collapse or expand the current file.

This removes some very complicated/messy code around following links in the table of contents and getting files auto-expanded. I suspect no one will miss this, but we can restore it if ayone notices.

Test Plan: Pressed "h" to collapse/expand a file. Also used the menu items.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12616

Differential Revision: https://secure.phabricator.com/D17940
2017-05-18 10:21:37 -07:00
Austin McKinley
1e3c8df1c8 Migrate Project names to modular transactions
Summary: Also changes access modifiers on `PhabricatorProjectTransactionEditor` and sets up `storage` for `applyExternalEffects`.

Test Plan: Created new projects, attempted to create without name, with too long of a name, and with a name that conflicts with other projects and observed expected errors.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: epriestley

Maniphest Tasks: T12673

Differential Revision: https://secure.phabricator.com/D17947
2017-05-17 17:12:22 -07:00
epriestley
80c329c942 When replying to a threaded inline, put the new inline in the right place in the thread
Summary:
Fixes T10563. If you have a thread like this:

```
> A
  > B
  > C
```

...and you reply to "B", we should put the new inline below "B".

We currently do when you reload the page, but the initial edit goes at the bottom always (below "C").

Test Plan: {F4963015}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10563

Differential Revision: https://secure.phabricator.com/D17938
2017-05-17 12:27:14 -07:00
Chad Little
75fb1a0327 Don't render an action list without actions
Summary: Skips rendering of partial elements if no actions are present.

Test Plan: Tested on profile menu item page, maniphest curtain, phriction dropdown, and instance backups page (no actions at all).

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D17931
2017-05-17 09:10:48 -07:00
epriestley
dde63af1cc Fix a JS console warning when hovering over replies to ghosts on lines which no longer exist
Summary:
Fixes T11662. In the very obscure situation described in that task, quiet a JS console warning.

The actual edit operation appears to work correctly after changes elsewhere.

There aren't really any legitimate lines for us to highlight in this case so I'm just giving up rather than trying to do something approximate.

Test Plan:
  - Wrote `long.txt`.
  - Created revision.
  - Added an inline near the bottom.
  - Removed most of `long.txt`.
  - Updated revsion.
  - Replied to the ghost inline.
  - Edited the reply to the ghost inline, worked.
  - Hovered the reply to the ghost inline: no line highlight, but no errors.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T11662

Differential Revision: https://secure.phabricator.com/D17930
2017-05-17 08:55:10 -07:00
epriestley
343f7cac72 Improve mobile/device behaviors for inline comments
Summary: Fixes T1026. Ref T12616. Allows drag-to-select on devices to add inlines on a range of lines, using dark magic that I copy/pasted from StackOverflow.

Test Plan: Left a comment on a range of lines on iPhone simulator.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12616, T1026

Differential Revision: https://secure.phabricator.com/D17928
2017-05-17 08:53:42 -07:00
epriestley
51df02821b Move the "select a line range" inline code to DiffInline
Summary: Ref T12616. This makes line range selection use the new code, and removes the remainder of the old "hover a line number" / "select a line range" code.

Test Plan: Hovered line numbers; selected line ranges.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12616

Differential Revision: https://secure.phabricator.com/D17927
2017-05-17 08:41:26 -07:00
epriestley
e4e91ebf6f In Differential, allow "r" to create comments and "R" to quote
Summary:
Ref T11401. Fixes T5232. Ref T12616.

Partly, this moves more code over to the new stuff.

This also allows "r" to work if you have code selected (not just comments). If you "reply" to code, you start a new comment.

You can "R" a comment to quote it. This just starts a new comment normally if you "R" a block of code. This is sort of a power-user version of "quote" since it seems like it probably doesn't really make sense to put it in the UI ever (maybe).

With the new click-to-select, you can click + "R" to reply-with-quote.

Test Plan: Used "r" and "R" to reply to comments and code.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12616, T11401, T5232

Differential Revision: https://secure.phabricator.com/D17920
2017-05-16 17:37:54 -07:00
epriestley
0ca49fbeb9 Move "hover over an inline to see the affected lines" code to the new class tree
Summary:
Fixes T8047. Ref T12616. Fixes T9270. This moves the "hover" part of the hover/drag behavior to the new code, leaving the "drag" part for a followup change.

The new hover UI behaves properly with Quicksand (T8047) and the filetree (T9270).

Test Plan: Hovered over inlines, saw lines select properly.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12616, T9270, T8047

Differential Revision: https://secure.phabricator.com/D17919
2017-05-16 17:37:38 -07:00
epriestley
772afc5ed8 Allow cancelled inlines, edits, and replies to be undone to get the text back again
Summary: Ref T12616. The ability to do {nav Edit > Cancel > Undo} to get your text back on inlines got dropped during the conversion. Restore it.

Test Plan:
Created, replied, and edited inlines, typed text, then cancelled. Was able to undo.

Also undid normal deletion.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12616

Differential Revision: https://secure.phabricator.com/D17916
2017-05-16 13:12:14 -07:00
epriestley
6a9dd61c42 Make collapsed inlines more useful and anchor target highlights more accurate
Summary:
Ref T12616. Fixes T11648. Currently, we snug up replies with a negative margin (from T10563) but this throws off the anchor highlighting.

Instead:

  - Remove padding from these dolumns.
  - Use margins on the stuff inside them instead.
  - Less margins for replies.
  - Less margins for collapsed comments.
  - Show some text for collapsed comments.

Test Plan:
{F4960890}

{F4960891}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12616, T11648

Differential Revision: https://secure.phabricator.com/D17913
2017-05-16 11:09:53 -07:00
epriestley
325682248a If there's an anchor in the URL in Differential, don't stick to the bottom of the page as content loads
Summary:
Fixes T11784. A lot of things are interacting here, but this probably gets slightly better results slightly more often?

Basically:

  - When we load content, we try to keep the viewport "stable" on the page, so the page doesn't jump around like crazy.
  - If you're near the top or bottom of the page, we try to stick to the top (e.g., reading the summary) or bottom (e.g., writing a comment).
  - But, if you followed an anchor to a comment that's close to the bottom of the page, we might stick to the bottom intead of staying with the anchor.

Kind of do a better job by not sticking to the bottom if you have an anchor. This will get things wrong if you follow an anchor, scroll down, start writing a comment, etc. But this whole thing is a pile of guesses anyway.

Test Plan:
  - Followed an anchor, saw non-sticky stabilization.
  - Loaded the page normally, scrolled to the bottom real fast, saw sticky stabilization.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T11784

Differential Revision: https://secure.phabricator.com/D17911
2017-05-16 11:06:52 -07:00
epriestley
8052ab84bf Remove "^" (Prev) and "V" (Next) actions on Differential inline comments
Summary:
Ref T12616. Fixes T12715. I suspect these are very rarely used. (I think you tried to get rid of them before but I pushed back since we couldn't really offer great alternatives at the time?)

Now that the code is in a better place:

  - Click an inline's header (just the colored part) to select it with the keyboard selection cursor.
  - Click again to deselect it.
  - You can use "n" and "p" to jump to comments, so "click + n" is the same as the old "V" action.
  - This also makes it easier to swap between keyboard and mouse workflows, since you can jump into things with the keyboard at any inline.

Also, make "Reply" render more consistently.

Test Plan:
  - Did all that stuff, things seemed to work OK.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12715, T12616

Differential Revision: https://secure.phabricator.com/D17908
2017-05-16 09:44:00 -07:00
epriestley
1b5a276a02 Add Differential keyboard shortcuts for "mark done" and "hide/show"
Summary:
Fixes T8130. Allows selected comments to be shown/hidden (with "q") or marked done/not-done (with "w").

(These key selections are because "qwer" are right next to each other on QWERTY keyboards, and now mean "hide, done, edit, reply".)

Also, allow "N" and "P" to do next/previous inline, including hidden inlines. This makes "q" to hide/show a little more powerful and a little easier to undo.

Test Plan: Used "q", "w", "N" and "P" to navigate and interact with comments.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T8130

Differential Revision: https://secure.phabricator.com/D17906
2017-05-16 08:23:22 -07:00
epriestley
a154407efb Retain keyboard cursor state across more inline edit operations in Differential
Summary:
Ref T12634. Fixes T8131. Currently, most edit operations (edit, reply, collapse, mark done) lose the keyboard cursor state.

Instead, bind the state more tighlty to the inline object itself (instead of the rows which happen to be in the document), and then do a bit of recalculation to try to keep it selected across edits.

Test Plan:
  - Used "n" to select an inline.
  - Clicked "Done" checkbox.
  - Pressed "n".
  - Went to the next inline (previously: lost position in document).
  - Behavior is also better for: edit, reply, collapse/expand.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12634, T8131

Differential Revision: https://secure.phabricator.com/D17905
2017-05-16 08:23:04 -07:00
epriestley
1493f08272 Emit resize events after making document changes during inline editing
Summary:
Ref T12634. Fixes T12633. These events allow the keyboard reticle to resize properly.

(I expect to possibly hide/disable the reticle in the future during edits, but at least make the behavior sensible for now.)

Test Plan:
  - Used "n" to select a block.
  - Clicked a line number in that block to start a new inline comment.
  - Saw reticle resize properly.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12634, T12633

Differential Revision: https://secure.phabricator.com/D17904
2017-05-16 08:22:43 -07:00
epriestley
665ff4fdf6 Redraw the Differential keyboard reticle after collapsing/un-collapsing an inline
Summary: Ref T12634. Fixes T10049. Toggling an inline currently leaves the reticle oddly-positioned.

Test Plan:
  - Selected a comment with the keyboard.
  - Collapsed it.
  - Saw reticle behave reasonably.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12634, T10049

Differential Revision: https://secure.phabricator.com/D17903
2017-05-16 08:22:24 -07:00
epriestley
3c0da81619 When the Differential filetree is toggled, resize the keyboard reticle properly
Summary:
Ref T9270. Ref T12634. Emit a resize event after toggling the filetree so that things can recalculate layout.

This just does the keyboard reticle, not the mouse/edit reticle.

Test Plan:
  - Used "n" to select a block.
  - Used "f" to toggle the filetree.
  - Saw reticle resize properly.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12634, T9270

Differential Revision: https://secure.phabricator.com/D17902
2017-05-16 08:22:07 -07:00
epriestley
7d6133929a Resize the Differential keyboard focus reticle when the window is resized
Summary: Fixes T12632. Ref T12634. Currently, the keyboard focus reticle does not redraw properly after a window resize.

Test Plan:
  - Used "n" to select a block.
  - Resized the window.
  - Saw the reticle also resize properly.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12634, T12632

Differential Revision: https://secure.phabricator.com/D17901
2017-05-16 08:21:32 -07:00
epriestley
5d7202526f Hide the Differential keyboard focus reticle after Quicksand navigation
Summary: Ref T8047. Ref T12634. When we sleep, hide the reticle. Restore it when we wake.

Test Plan:
  - With Quicksand enabled..
  - Used "j" to select a change in a revision.
  - Navigated away by clicking a link.
  - WOW! Reticle vanished properly!
  - Used "back" to return.
  - Reticle returned properly.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12634, T8047

Differential Revision: https://secure.phabricator.com/D17900
2017-05-16 08:21:11 -07:00
Chad Little
1e47ba2481 Use setDrag UI for reordering workboard columns
Summary: This UI can use the setDrag call to reduce clutter on the reodering dialog.

Test Plan:
Reorder some columns, save.

{F4959906}

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D17898
2017-05-16 07:13:25 -07:00
epriestley
2fb1edfeb1 Restore the Differential "edit" and "reply" keyboard shortcuts
Summary: Ref T12616. This makes "edit" and "reply" work again.

Test Plan:
Used "e" and "r" to edit and reply.

Also used them in bogus ways and got useful UI feedback.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12616

Differential Revision: https://secure.phabricator.com/D17895
2017-05-16 06:25:35 -07:00
epriestley
588a66c04d Move most Differetial keyboard shortcuts into DiffChangesetList
Summary: Ref T12616. This moves most keyboard shortcuts into DiffChangesetList. It breaks some shortcuts that I plan to restore later, noted in T12616 (toggle file, edit inline, reply to inline), since I think ripping them out now and rebuilding them in a little bit will make things much simpler.

Test Plan:
  - Used j, k, n, p, J, K shortcuts to navigate a revision.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12616

Differential Revision: https://secure.phabricator.com/D17859
2017-05-16 06:24:42 -07:00
epriestley
41379f39de Move inline replies to new code and remove DifferentialInlineEditor
Summary:
Ref T12616. This moves "reply" to the new stuff and deletes DifferentialInlineEditor, which no longer does anything.

(This breaks some keyboard shortcuts, but I'll rebase D17859 shortly.)

Test Plan: Replied to inlines; things seemed to work properly.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12616

Differential Revision: https://secure.phabricator.com/D17894
2017-05-16 06:23:51 -07:00
epriestley
58dded555b Move inline comment creation to new DiffInline code
Summary:
Ref T12616. This makes creating inlines use the new code.

Creation and editing is now slightly more consistent in how it uses nodes. This will simplify the next change (replies), which I ran into some trouble with in an earlier iteration.

Note that this (and other changes in the series) allow you to create and edit multiple inlines simultaneously. This is mostly a feature, although I expect we'll need to lock it down a little bit. I have some UI ideas to help avoid errors.

Test Plan: Created inlines on a single line; on a range of lines; on the same line; multiple inlines at the same time.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12616

Differential Revision: https://secure.phabricator.com/D17893
2017-05-16 06:22:00 -07:00
epriestley
d97f80bc90 Make new DiffInline code handle most "delete" operations
Summary:
Ref T12616. This moves the delete actions to the new, more stateful way of doing things.

These are a little tricky because you can click "Delete" on an inline, but you can also click "Delete" from the preview area at the bottom of the page. If you do, the inline you are deleting may or may not be present on the page.

This has a few bugs -- notably, deleting from the preview without interacting with the on-page inline first won't actually delete the on-page inline yet -- but nothing too serious.

Test Plan: Deleted inlines, undid deletion, deleted from preview.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12616

Differential Revision: https://secure.phabricator.com/D17890
2017-05-16 06:21:37 -07:00
epriestley
3c18cb77fb Move inline "done" checkboxing to DiffInline
Summary:
Ref T12616. This updates clicking the "Done" checkbox for the new stuff.

This one is pretty clean since the "Done" checkbox doesn't do too much weird magic.

Test Plan: Clicked the box a few times.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12616

Differential Revision: https://secure.phabricator.com/D17888
2017-05-16 06:21:00 -07:00
epriestley
798c8ba696 Mostly move inline editing to DiffInline
Summary:
Ref T12616. This doesn't pull over everything (some UI feedback didn't make it yet, and you can't cancel + undo cancelling edits yet) but editing comments technically works.

This is a little shaky, but feels less shaky than every other approach I've tried, so I think I'm finally on a reasonable track here.

Test Plan: Edited some inline comments.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12616

Differential Revision: https://secure.phabricator.com/D17887
2017-05-16 06:20:26 -07:00
epriestley
4fd4ec3d27 Hide inlines one-by-one, instead of in a big group
Summary:
Ref T12616. Fixes T12153. Currently, when you hide inlines, they hide completely and turn into a little bubble on the previous line.

Instead, collapse them to a single line one-by-one. Narrowly, this fixes T12153.

In the future, I plan to make these changes so this feature makes more sense:

  - Introduce global "hide everything" states (T8909) so you can completely hide stuff if you want, and this represents more of a halfway state between "nuke it" and "view it".
  - Make the actual rendering better, so it says "epriestley: blah blah..." instead of just "..." -- and looks less dumb.

The real goal here is to introduce `DiffInline` and continue moving stuff from the tangled jungle of a million top-level behaviors to sensible smooth statefulness.

Test Plan:
  - Hid and revealed inlines in unified and two-up modes.
  - These look pretty junk for now:

{F4948659}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12616, T12153

Differential Revision: https://secure.phabricator.com/D17861
2017-05-16 06:19:56 -07:00
epriestley
fe44e987fb Translate "Loading..." text in inline comments
Summary: Ref T12616. This cements the relationship between ChangesetList (parent container) and Changeset (child) and passes translations down so Changeset can use them to translate the text "Loading..."

Test Plan: Viewed loading changes.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12616

Differential Revision: https://secure.phabricator.com/D17846
2017-05-16 06:19:01 -07:00
epriestley
64a54aac9d Merge "differential-dropdown-menus" behavior into DiffChangesetList
Summary: Ref T12616. This ends up being a little messy ("one giant function") and maybe I'll clean it up a bit later, but continue consolidating the wild jungle of behaviors into a smaller set of responsible objects.

Test Plan: Clicked all the menu options, saw them work properly. Grepped for removed methods.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12616

Differential Revision: https://secure.phabricator.com/D17845
2017-05-16 06:18:26 -07:00
epriestley
63450cc48e Remove "Show All Context" button from Diffusion
Summary:
Ref T12616. Diffusion, only, has a "Show All Context" button which expands the full context on all changes.

I don't remember the exact history on this, but it hasn't existed in Differential for some time and no one has complained. I suspect that the "View Options > Show All Context" on each file may replace it. I can't really come up with good reasons to use it, offhand. If we want to restore it, I think global options after T1591 is promising.

{F4945561}

Test Plan:
  - Loaded a commit in Diffusion, no longer saw a button.
  - Grepped for relevant sigils.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12616

Differential Revision: https://secure.phabricator.com/D17843
2017-05-16 06:17:52 -07:00
epriestley
2bd25d7399 Rename "DifferentialChangesetViewManager" to "DiffChangeset"
Summary: Ref T12616. This class is already mostly-reasonable as a representation of an individual changeset, so I plan to just adjust it a little bit.

Test Plan:
  - Used `git grep` to search for `ChangesetViewManager`.
  - Used `git grep` to search for `changeset-view-manager`.
  - Browsed around and interacted with changesets.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12616

Differential Revision: https://secure.phabricator.com/D17842
2017-05-16 06:16:49 -07:00
epriestley
993d942117 Gently move some listeners into DiffChangesetList
Summary: Ref T12616. Put these listeners in DiffChangesetList so the wake/sleep properly for Quicksand.

Test Plan:
  - Added some logging.
  - With quicksand, moved between diffs.
  - Saw "load" and "show more" fire exactly once on each page, with the correct changeset list listener.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12616

Differential Revision: https://secure.phabricator.com/D17841
2017-05-16 06:16:26 -07:00
epriestley
404fe482d9 Add a Quicksand-aware page-level container to diffs
Summary:
Ref T12616. Ref T8047. Ref T11093. We currently have several bugs where diff state sticks across Quicksand pages.

Add a new top-level object to handle this, with `sleep()` and `wake()` methods. In `sleep()`, future changes will remove/deacivate all the reticles/editors/etc.

See T12616 for high-level discussion of plans here.

This general idea is likely to become more formal eventually (e.g. for "sheets" or whatever we call them, in T10469) but I think this is probably a reasonable place to draw a line for now.

Test Plan:
  - Added some logging to sleep(), wake() and construct().
  - Viewed changes in Differential.
  - With Quicksand on, browsed around; saw state change logs fire properly.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12616, T11093, T8047

Differential Revision: https://secure.phabricator.com/D17840
2017-05-16 06:15:58 -07:00
Chad Little
f600bc0811 Clean up watchers and members project page
Summary: Various little fixes, mostly moves information from the "Details" section either into the curtain or into the specific watchers or members list based on user viewership. I think this page is both cleaner and more informative.

Test Plan:
Lock, Unlock, Watch, Join, various projects with multiple users.

{F4959101}

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D17891
2017-05-15 15:56:14 -07:00
Chad Little
d6a620be45 Update Maniphest for modular transactions
Summary: Ref T12671. This modernized Maniphest transactions to modular transactions.

Test Plan:
- Create Task
- Edit Task
- Raise Priority
- Change Status
- Merge as a duplicate
- Create Subtask
- Claim Task
- Assign Project
- Move on Workboard
- Set a cover image
- Assign story points
- Change story points
- Generate lots via lipsum
- Bulk edit tasks
- Leave comments
- Award Token

I'm sure I'm missing something.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: hazelyang, Korvin

Maniphest Tasks: T12671

Differential Revision: https://secure.phabricator.com/D17844
2017-05-15 10:29:06 -07:00
Austin McKinley
0dc90b891a Reimplement Slowvote transactions using modular transactions
Summary:
Fixes T12623. Adds new modular transactions to Slowvote. Also converts
the `shuffle` column to `bool` for consistency with other boolean-ish columns.

Test Plan:
Create a new vote, modified everything that could be modified from the web UI,
observed expected timeline.

Example timeline: {F4938843}

Example transaction values in DB: {F4938850}

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T12623

Differential Revision: https://secure.phabricator.com/D17830
2017-05-04 20:20:00 -07:00
Chad Little
3d328512d2 Use a lighter color for completed list items in remarkup
Summary: In remarkup lists, it can be hard to clearly see which items still need to be completed. This makes completed items a little lighter for clarity.

Test Plan:
Review a long list with checked and unchecked items in a task.

{F4938611}

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D17828
2017-05-04 12:45:13 -07:00
Chad Little
d6bce34a5d Update Conpherence to use EditEngine
Summary: Moves Conpherence to use EditEngine. This removes the "First Message" field, but I think that's ok until we have direct messaging of some sort, then maybe have built-ins cover that case.

Test Plan:
 - Visit /new/ and /edit/ for creating new rooms.
 - Edit a room in full conpherence
 - Edit a room in durable column
 - grep for METADATA calls

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T11729

Differential Revision: https://secure.phabricator.com/D16677
2017-04-29 19:39:12 -07:00
epriestley
d2baa88171 Allow Owners packages to have arbitrarily long names
Summary:
  - Change column type from `sort128` to `sort`.
  - Remove `originalName`. This column is unused. Long ago, we used it to generate a `Thread-Topic` header for mail, but just use PHIDs now (the value just needs to be stable for a given object, users normally don't see it).

Test Plan:
  - Created a package with a beautifully long name. Magnificent!
  - Grepped for `originalName` / `getOriginalName()`, found no Owners hits.
  - Verified that there isn't any name-length validation code to remove.

{F4925637}

Reviewers: chad, amckinley

Reviewed By: chad

Differential Revision: https://secure.phabricator.com/D17798
2017-04-27 18:04:03 -07:00
epriestley
6da73fb361 Leafy fresh vegetables. 2017-04-26 08:49:53 -07:00
Chad Little
d70d09c02c Fix notice spacing on table views
Summary: When a notice is in a table view in a two column layout, reset the margins.

Test Plan: Visit OwnerDetails with no paths set.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D17792
2017-04-25 16:46:55 -07:00
Chad Little
279816683b Move ToC 4px in documentproview
Summary: Fixes T12637.

Test Plan: Reload page, see wider button.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T12637

Differential Revision: https://secure.phabricator.com/D17790
2017-04-25 10:50:34 -07:00
Chad Little
e18ce14c20 Fix closed task hovercard background color
Summary: Ref T12600. This makes the background solid when in hovercard context.

Test Plan: Close a task, view hovercard, see solid background

Reviewers: epriestley, amckinley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T12600

Differential Revision: https://secure.phabricator.com/D17788
2017-04-25 08:40:29 -07:00
epriestley
e2a94019b1 Migrate accounts to correct user email verification state flag
Summary:
Depends on D17785. Fixes T12635. There was a bug where users could verify their primary email without getting the "isEmailVerified" flag set on their accounts.

D17785 fixes this bug. This change migrates affected account to fix their state, now that they can't get in trouble any more (hopefully).

Test Plan:
  - Explicitly removed this flag from a bunch of accounts.
  - Ran migration, saw the accounts get fixed.
  - Ran migration again (`storage upgrade --apply ...`), saw the accounts not get touched.
  - We have 117 affected accounts on `secure`, so I'll verify that this fixes them.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12635

Differential Revision: https://secure.phabricator.com/D17786
2017-04-24 10:24:51 -07:00
epriestley
41d9453ece When a user changes to a verified primary address, mark their account as verified
Summary:
Ref T12635. See that task for discussion.

You can currently end up with a verified primary address but no "verified" flag on your account through an unusual sequence of address mutations.

Test Plan:
  - Registered without verifying, using address "A".
  - Added a second email address, address "B".
  - Verified B (most easily with `bin/auth verify`).
  - Changed my primary email to B.
  - Before patch: account not verified.
  - After patch: account verified.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12635

Differential Revision: https://secure.phabricator.com/D17785
2017-04-24 10:24:21 -07:00
Chad Little
b3c8226cbb New hovercard UI for Maniphest Tasks
Summary: Swaps out hovercard boring view for super cool workboard card view. Will have more diffs to add additional information down the road.

Test Plan: {F4921092}

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D17769
2017-04-23 18:58:08 -07:00
epriestley
bef68ccdd6 Regenrate Quickstart SQL
Summary:
The quickstart SQL hasn't been regenrated in a while, mildly impacting unit test and instance startup times.

  - Use `bin/storage quickstart` to regenerate quickstart.
  - Manually set the FULLTEXT tables back to `MyISAM` until we deal with T11741.

Test Plan:
  - Saw database setup drop from ~10,500ms to ~7,500ms locally.
  - Visually inspected diff, changes looked expected.

Reviewers: chad

Reviewed By: chad

Differential Revision: https://secure.phabricator.com/D17775
2017-04-23 11:04:02 -07:00
epriestley
52c4715bbc No-op the Conpherence thumbnail resizing migration
Summary:
Fixes T12628. After later changes to `PhabricatorFile`, this migration no longer runs if you upgrade through it to a recent `HEAD` while your data has some room images.

Since this isn't critical and has been available for ~6 months, I just nuked it as a first pass. I can find a more careful approach which lets us continue to run this migration instead if you're hesitant to skip this step, although it may be a little involved.

In 95% of cases we avoid this by updating the storage table as it existed at the time the migraiton ran, but Files are much too complicated for that to be realistic.

Test Plan: Ran `bin/storage upgrade -f --apply phabricator:20161005.conpherence.image.2.php`, saw it do nothing.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12628

Differential Revision: https://secure.phabricator.com/D17770
2017-04-23 10:46:09 -07:00
Chad Little
ede23efcc7 Punch up grey button border grey 10%
Summary: Visually these are hard to see on blue backgrounds, adds a touch more contrast. Fixes T12604

Test Plan: View as pager and dialog

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T12604

Differential Revision: https://secure.phabricator.com/D17755
2017-04-21 11:43:43 -07:00
Chad Little
d3546f94c1 Improve diffusion readme layout
Summary: Uses more standard objects and more padding for reading. Removes the ToC, which is visually broken anyways.

Test Plan: Review a README.md in a local repository.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D17752
2017-04-21 11:23:26 -07:00
Chad Little
7c61ace086 Attach Diffusion Pagers to their ObjectBoxView
Summary: Adds the ability to set a pager onto an object box directly and pick up appropriate styles.

Test Plan: grep for renderTablePagerBox, test layouts with and without a pager.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T12604

Differential Revision: https://secure.phabricator.com/D17754
2017-04-21 11:22:19 -07:00
Chad Little
050538cf7e Center icons on DiffusionCloneURIView
Summary: These icons are off center currently.

Test Plan: Review a clone uri in local repository.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D17753
2017-04-21 10:55:11 -07:00
Chad Little
3a608bae7b Better built in images
Summary: iam

Test Plan: iam

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D17751
2017-04-21 08:58:50 -07:00
Chad Little
ed9afa18cc Allow users to choo choo choose a room color
Summary: This adds some basic per user / per room theming for Conpherence, which should hopefully let users identify rooms from just the sidebar color.

Test Plan: Lots of threads with different colors.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D17747
2017-04-20 14:30:59 -07:00
epriestley
1c222c8834 In Conpherence, stop throwing away stuff users have typed when a reply arrives
Summary:
Ref T12562. I think the pre-send-on-enter behavior was: disable textarea, send message, clear area on response?

That got changed but not completely, maybe. There's currently an issue here:

  - Add a `sleep(3)` to `UpdateController`.
  - Type "AAA".
  - Press enter.
  - Real fast, type "BBB".
  - When the "AAA" arrives, your "BBB" is lost. Sad!

Test Plan:
  - Did the thing described above; no longer lost "BBB".
  - Switched threads, sent messages, couldn't find anything else this breaks. It dates from a long time ago so I think it's just pre-SOE stuff.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12562

Differential Revision: https://secure.phabricator.com/D17742
2017-04-20 13:07:24 -07:00
epriestley
d2560b2462 Make "Locate File" trigger when data loads if the user typed/pasted real fast
Summary:
Fixes T12599. If you're faster than the network request, we don't actually resolve your query when the data arrives.

Instead, when the data shows up, run the query if the user has typed something.

Test Plan: Pasted a filename in real fast, got results. (Previously: no results until you press something else.)

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T12599

Differential Revision: https://secure.phabricator.com/D17739
2017-04-19 16:14:48 -07:00
epriestley
95dd9dbf43 Make Applications extend LiskDAO
Summary:
Ref T11476. This is a bit hacky, but makes `Application` extend `LiskDAO` so we can apply transactions to it with an `Editor` class.

Also fixes schema stuff so builds should produce a clean bill of health again.

This might only get you slightly further, yell if you run into more trouble.

Test Plan:
  - Ran `bin/storage upgrade -f` and got no warnings.
  - Browsed around, nothing exploded?

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T11476

Differential Revision: https://secure.phabricator.com/D17738
2017-04-19 16:06:14 -07:00
Austin McKinley
febd68039f Add initial infrastructure for adding ModularTransaction support to Application config changes
Summary: Part of the groundwork for T11476.

Test Plan: ran `./bin/storage upgrade` and observed expected DB tables

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T11476

Differential Revision: https://secure.phabricator.com/D17736
2017-04-19 15:44:57 -07:00
epriestley
76d0b67d91 Remove "dateTouched" from ConpherenceParticipant
Summary:
Pathway to D17685. This column is (mostly) a denormalization of `dateModified` on the thread.

Just use a JOIN instead.

This isn't //exactly// the same: we'll bump threads to the top now for non-message changes (e.g., a topic or title change). That seems fine, but we could put a `lastMessageDate` on Thread later if we want to refine it.

Also got rid of a lot of other unused stuff. There's a big garbage TODO here, I'll fix that in the next change.

Test Plan:
  - Grepped for `dateTouched`.
  - Grepped for `participantCursor`.
  - Grepped for `ConpherenceParticipantQuery::LIMIT`.
  - Looked for callsites to `setOrder()`, found none.
  - Added a message to an older thread, saw it bump up to the top.

Reviewers: chad

Reviewed By: chad

Differential Revision: https://secure.phabricator.com/D17731
2017-04-19 13:58:54 -07:00
epriestley
0a335f91cd Remove "participationStatus" from ConpherenceParticipant
Summary:
Pathway to D17685. This column is a very complicated cache of: is participant.messageCount equal to thread.messageCount?

We can just ask this question with a JOIN instead and simplify things dramatically.

Test Plan:
  - Ran migration.
  - Browsed around.
  - Sent a message, saw unread count go up.
  - Read the message, saw unread count go down.

Reviewers: chad

Reviewed By: chad

Differential Revision: https://secure.phabricator.com/D17730
2017-04-19 13:58:42 -07:00
epriestley
c96e17f5ea Remove "behindTransactionPHID" from ConpherenceParticipant
Summary: Pathway to D17685. Nothing reads this field and it has no use or value.

Test Plan:
  - Ran migration.
  - Grepped for `behindTransactionPHID`.

Reviewers: chad

Reviewed By: chad

Differential Revision: https://secure.phabricator.com/D17729
2017-04-19 13:58:29 -07:00
Chad Little
15e7624a17 Add a few more sounds
Summary: A few more mp3s to choose from for Conpherence.

Test Plan: Test each sound in a new room.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D17734
2017-04-19 13:47:23 -07:00
Chad Little
df7f56d8e3 Minor CSS tweaks Conpherence
Summary: Minor pixel shifts with new header ui in place.

Test Plan: Desktop, Mobile, Tablet, with and without search and participants open

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D17725
2017-04-18 14:10:11 -07:00
epriestley
ab2aa74d6e Fix several duplication/replay behaviors in Aphlict
Summary:
Ref T12566. Ref T12563. This fixes three bugs with Aphlict replay stuff:

First, Conphernece would try to repaint the UI even if no thread was open. Only repaint when a thread is open.

Second, although we deduplicate JX.Leader messages, we didn't deduplicate actual notification messages. If you browsed the leader window, then it re-elected itelf as a leader and replayed history, it could rebroadcast notifications and other windows could show doubles. Deduplicate notifications to prevent this.

Third, we always replayed the last 60 seconds of history. When you browsed the leader window, whichever window became the new leader (possibly the one you just browsed) could replay messages from before it had opened, leading to duplicate messages. Particularly, after receiving a message and then browsing you could see that message again. Instead, only replay history as far back as when the window first opened.

Test Plan:
  - Clicked "Repaint" with a thread open, saw a repaint. Clicked "Repaint" with Conpherence open but no thread, no repaint and no 404 request to `/update/null/`.
  - In browser A, opened three windows. In browser B, sent a notification. In browser A, browsed the leader window away twice in a row. Observed that the window which never became a leader doesn't duplicate notifications.
  - In browser A, opened three windows. In browser B, sent a notification. In browser A, browsed the leader window away over and over again. Observed that replay requests issued with appropriate history windows.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12566, T12563

Differential Revision: https://secure.phabricator.com/D17722
2017-04-18 12:10:12 -07:00
epriestley
5d55804e3f Play a sound when receiving a new chat message
Summary:
Ref T7567. Nothing fancy yet, just getting this working. Sound is lightly edited version of "Pop 6":

https://www.freesound.org/people/greenvwbeetle/sounds/244656/

Test Plan: Sent chat, heard sounds.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T7567

Differential Revision: https://secure.phabricator.com/D17721
2017-04-18 11:34:17 -07:00
Austin McKinley
be00264ae7 Make daemons perform file deletion
Summary:
Deletion is a possibly time-intensive process, especially with large
files that are backed by high-latency, chunked storage (such as
S3). Even ~200mb objects take minutes to delete, which makes for an
unhappy experience. Fixes T10828.

Test Plan:
Delete a large file, and stare in awe of the swiftness with
which I am redirected to the main file application.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: thoughtpolice, Korvin

Maniphest Tasks: T10828

Differential Revision: https://secure.phabricator.com/D15743
2017-04-18 11:09:41 -07:00
epriestley
e187519f00 When a transaction has no quote ref, render "@user wrote:" properly
Summary:
Fixes T12576. In Javascript, `data.ref` is null, which is getting turned into `/quote/?ref=null`.

The code already handles this case, just not with `ref=null` happening in JS:

https://secure.phabricator.com/source/phabricator/browse/master/src/applications/transactions/controller/PhabricatorApplicationTransactionCommentQuoteController.php;b54adc6161c205e146fabb801ca53a44d94da444$47-52

Test Plan:
{F4913862}

  - Also quoted a normal comment on a normal object in a normal way.

Reviewers: amckinley, chad

Reviewed By: chad

Maniphest Tasks: T12576

Differential Revision: https://secure.phabricator.com/D17720
2017-04-18 09:51:53 -07:00
epriestley
7fd98a5e86 Every so often, ask the Aphict server how things are going
Summary: Ref T12573. This sends a "ping" to the server, and a "pong" back to the client, every 15 seconds. This tricks ELBs into thinking we're doing something useful and productive.

Test Plan: Ran `bin/aphlict debug`, loaded Phabricator, saw ping/pong in logs.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12573

Differential Revision: https://secure.phabricator.com/D17717
2017-04-17 20:33:43 -07:00
Austin McKinley
976fbee877 Implement ngram search for File objects
Summary: Follows the outline in D15656 for implementing ngram search for names of File objects. Also created FileFullTextEngine, because without implementing `PhabricatorFulltextInterface`, `./bin/search` complains that `File` is not an indexable type.

Test Plan:
  - ran `./bin/storage upgrade` to apply the schema change
  - confirmed the presence of a new `file_filename_ngrams` table
  - added a couple file objects
  - ran `bin/search index --type file --force`
  - confirmed the presence of rows in `file_filename_ngrams`
  - did a few keyword searches and saw expected results

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T8788

Differential Revision: https://secure.phabricator.com/D17702
2017-04-17 17:37:20 -07:00
epriestley
ffed156981 After a reconnect, repaint Conpherence thread state
Summary: Ref T12566. When we reconnect, refresh the current thread even if we replayed notifications.

Test Plan:
  - Clicked the "Repaint" button, saw the thread refresh.
  - Clicked the "Reconnect" button, saw the thread reresh.
  - Launched `aphlict debug`, killed it, restarted it, saw the thread refresh after reconnect.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12566

Differential Revision: https://secure.phabricator.com/D17713
2017-04-17 16:00:32 -07:00
epriestley
eaecf35324 Deduplicate application-level notifications from Aphlict
Summary:
Fixes T12564. We already had some code which seems to deal with this properly, it just wasn't getting used.

Assign each application-level notification a unique ID, then ignore messages with duplicate IDs.

Test Plan:
  - In browser A, loaded `/T123`.
  - In browser B, loaded `/T123`.
  - Made a comment as B.
  - Saw notification as A.
  - Mashed "Replay" a bunch.
  - Before patch: piles of duplicate notifications.
  - After patch: no duplicates.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12564

Differential Revision: https://secure.phabricator.com/D17710
2017-04-17 15:55:38 -07:00
epriestley
02194f0fc8 After Aphlict reconnects, ask the server to replay recent messages
Summary:
Fixes T12563. If we've ever seen an "open", mark all future connections as reconnects. When we reconnect, replay recent history.

(Until duplicate messages (T12564) are handled better this may cause some notification duplication.)

Also emit a reconnect event (for T12566) but don't use it yet.

Test Plan: {F4912044}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12563

Differential Revision: https://secure.phabricator.com/D17708
2017-04-17 15:54:51 -07:00
epriestley
8fdc1bff5f When disconnected from Aphlict after a successful connection, retry the first reconnect right away
Summary:
Fixes T12567. We currently retry after 2s, 4s, 8s, 16s, ...

If we connected cleanly once, retry the first time right away. There are a bunch of reasonable cases where this will work fine and we don't need to wait. Then we fall back: 0s, 2s, 4s, 8s, ...

Test Plan: {F4911905}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12567

Differential Revision: https://secure.phabricator.com/D17706
2017-04-17 15:53:29 -07:00
epriestley
1212047843 Add a "Reconnect" debugging action and show reconnect delays in the console
Summary: Ref T12568. Ref T12567. Allows you to force a reconnect, and shows the reconnect delay on connection close/failure.

Test Plan: {F4911879}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12568, T12567

Differential Revision: https://secure.phabricator.com/D17705
2017-04-17 15:51:24 -07:00
epriestley
28c68eb4fd Decrease JX.Leader lease duration from 16,000ms to 1,500ms and usurp more aggressively
Summary:
Ref T12573. `JX.Leader` synchronizes the Aphlict connection across multiple windows.

Currently, we only test to see if the leader window has been closed every 16 seconds. Instead, test every 1.5 seconds.

Also, make windows keep trying to become the leader forever. This was removed previously (in D15806) but I think that change decreased robustness here.

Test Plan:
  - Opened two windows to the "Realtime" tab in DarkConsole.
  - Saw one become the leader and one become a follower.
  - (Optionally, wait for 10 seconds here to test the "keep trying to become the leader" behavior.)
  - Closed the leader.
  - Saw the follower become the leader after ~1.5 seconds.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12573

Differential Revision: https://secure.phabricator.com/D17703
2017-04-17 15:48:47 -07:00
Chad Little
3f45defd34 Clean up Remarkup Preview on mobile
Summary: Some space is bleeding in here from two-column-css. Re-scope CSS.

Test Plan: Review creating a task on mobile with document preview present.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D17712
2017-04-17 22:11:08 +00:00
Chad Little
2a5dae4fcb Minor Topic CSS tweaks
Summary: Cleans up the topic UI a little more, I think this feels nice for some reason.

Test Plan: visit a room with and without a topic, desktop, tablet, mobile

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D17711
2017-04-17 15:10:18 -07:00
Chad Little
ecbeec35b2 Tweak some icon buttons
Summary: Fixes T12577. Some tweaks last week widened the default buttons, but these didn't get retouched.

Test Plan: Review calendar on desktop and mobile.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T12577

Differential Revision: https://secure.phabricator.com/D17709
2017-04-17 14:36:59 -07:00
Chad Little
2d00f56837 Use PHUIListItemView in ConpherenceThreadList
Summary: Fixes T12556 Uses more common components in ConpherenceThreadList by moving to PHUIListItemView. Reduces clutter by moving privacy into the header. Gets ride of "See More" double interchanges.

Test Plan:
I need to test this more, doesn't seem to auto-select top room any more, also might build a lipsum generator.

 - Create lots of rooms with various policies
 - Test clicking on policy object
 - Click on different rooms
 - Post in rooms
 - Load up second account, see room numbers
 - Clear room message count by clicking on room

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T12556

Differential Revision: https://secure.phabricator.com/D17698
2017-04-17 11:21:49 -07:00
epriestley
b08b4cf0b5 Fix a bad variable in global typehaead order/limit code
Summary: Ref T12538. I missed this in D17695, which renamed the variable. The logic was also a little off since `jj` is an index, not a count.

Test Plan: Typed `con` in global search, which hits "Con-pherence", "Con-duit" and "Con-fig", plus a bunch of other stuff. Got results after patch.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12538

Differential Revision: https://secure.phabricator.com/D17700
2017-04-17 07:37:41 -07:00
epriestley
f0fbf7a7d3 Sort "closed" results (like disabled users) to the bottom of typeaheads, but don't hide them completely
Summary:
Fixes T12538. Instead of hiding "closed" results unless only closed results match, show closed results but sort them to the bottom.

This fixes the actual issue in T12538, and I think this is probably the correct/best behavior for the global search.

It also makes all other typeaheads use this behavior. They currently have a "bug" where enabled user `abcd` makes it impossible to select disabled user `abc`. This manifests in some real cases, where enabled function `members(abc)` makes it impossible to disabled user `abc` in some function tokenizers.

If ths feels worse, we could go back to filtering in the simpler cases and introduce a rule like "show closed results if only closed results would be shown OR if query is an exact match for the disabled result", but that gets dicier because "exact match" is a fuzzy concept.

(There are a lot of other minor bad behaviors that this doesn't try to fix.)

Test Plan:
Enabled project "instabug" no longer prevents bot user "instabug" from being shown:

{F4903843}

Disabled user "mmaven" is sorted below enabled user "mmclewis", in defiance of the otherwise alphabetical order. There's no visual cue that this user is disabled because of T6906.

{F4903845}

Same as above, but this source renders "disabled" in a more obvious way:

{F4903848}

Function selecting members of active project `members(instabug)` no longer prevents selection of bot user `instabug`:

{F4903849}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12538

Differential Revision: https://secure.phabricator.com/D17695
2017-04-14 13:01:48 -07:00
Chad Little
5587abf04c Remove recentParticipants from ConpherenceThread
Summary: We no longer display this any more in the UI, so go ahead and remove the callsites and db column.

Test Plan: New Room, with and without participants.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D17683
2017-04-13 13:55:08 -07:00
Chad Little
4189eb810b Use violet with not-verified user tags
Summary: Will see how this goes in practice. Uses violet where color is used for non responsive peeps.

Test Plan: Create a user without email verification, test hover card, profile, mentions and lists.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D17678
2017-04-13 12:19:49 -07:00
Chad Little
2c5ee2a225 Fix Durable Column CSS-Overload
Summary: This moves the count on the Conpherence Menu Item into a phui-list-item-count, and removes the CSS call to the entire Conphrence stack when durable column is open.

Test Plan: Test with and without the chat column, and a menu with a count

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D17677
2017-04-13 11:29:30 -07:00
Chad Little
03f2a41b16 Clean up Conpherence Transactions and notifications
Summary: Does a few things. Turns off feed stories (again), removes "action" transactions from notificiations, and only updates message count on actual messages. This feels a bit cleaner and less spammy... I guess... I think @epriestley will really like it and do me a favor or something.

Test Plan: Pull up two windows. test a message, see message count on second screen. Edit a topic or title, get no notification. At all. Ever.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D17674
2017-04-13 07:20:15 -07:00
epriestley
3245e74f16 Show users how fulltext search queries are parsed and executed; don't query stopwords or short tokens
Summary:
Depends on D17670. Fixes T12137. Fixes T12003. Ref T2632.

This shows users a readout of which terms were actually searched for.

This also drops those terms from the query we submit to the backend, dodging the weird behaviors / search engine bugs in T12137.

This might need some design tweaking.

Test Plan: {F4899825}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12137, T12003, T2632

Differential Revision: https://secure.phabricator.com/D17672
2017-04-12 19:07:54 -07:00
Chad Little
a7ebfc12c0 Modernize Conpherence with Modular Transactions
Summary: Begin converting Conpherence to ModularTransactions, this converts title, topic, and picture to use modular transactions. Participants seems hairy so I'll do that in another diff

Test Plan: Create a room with a topic, change room name, topic. Add people, remove people. Set a room image. Unset topic.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D17668
2017-04-12 16:33:57 -07:00
Chad Little
a9845b0b1d Remove picture crop transaction from Conpherence
Summary: Fixes T11730. Removes an old transaction that hasn't been used in a year.

Test Plan: Run sql, check various rooms.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T11730

Differential Revision: https://secure.phabricator.com/D17666
2017-04-12 14:18:54 -07:00
Chad Little
75303567b3 Add a Conpherence Profile Menu Item
Summary: Builds a Conpherence Profile Menu Item, complete with counts for the unreads. This allows pinning to home as well as swapping out thread list in Conpherence for pinning eventually.

Test Plan: Add a menu item, chat in room, log into other account, see room count. Room count disappears after viewing.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D17662
2017-04-12 13:07:44 -07:00
Chad Little
578ebe6c2f Clean up minor form spacing
Summary: Drops the inset box shadow and bumps standard UI elements from 28px to 30px. more room for activities.

Test Plan:
Spaces, Editing tasks, typeaheads, mobile, desktop.

{F4897792}

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D17660
2017-04-12 07:40:57 -07:00
epriestley
5cf53f7b3b Fix some minor curtain overflow/wrap issues
Summary:
Fixes T12503.

  - Users with creative usernames like `MMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMM` could overflow "Subscribers" in the curtain UI.
  - Other content like packages could also overflow.
  - Users with interesting and unique names like `WWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWW` who were also away or disabled could get a linebreak between their availability dot and their username.

Test Plan:
See T12503 for "before" screenshots. Also tested mobile, which looked fine, but didn't screenshot it.

{F4849900}

{F4849912}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12503

Differential Revision: https://secure.phabricator.com/D17650
2017-04-10 16:02:00 -07:00
Chad Little
4a84954957 Prevent Send on Enter in Fullscreen Remarkup Mode
Summary: Fixes T12138. Test for the presence of being in fullscreen mode, and disable send on enter if present. Side note, I'd love a first class "hasClass" type Javelin function.

Test Plan:
- Go to Conpherence
- Type some smack, see it send on enter
- Go fullscreen like a boss
- Let the words flow
- Close fullscreen, then send on enter.
- (might be nice someday to add a "submit" button to fullscreen editor)

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T12138

Differential Revision: https://secure.phabricator.com/D17590
2017-04-10 14:39:50 -07:00
epriestley
d4d46501ed Add a repair migration to fix legacy Calendar events without "utcInstanceEpoch"
Summary: Fixes T12488. Some events appear to have survived earlier migrations without getting completely fixed. Fix them.

Test Plan:
  - Ran migration locally with `bin/storage upgrade` (but: I could not reproduce this problem locally).
  - Ran migration in production and saw ICS import stop fataling.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12488

Differential Revision: https://secure.phabricator.com/D17642
2017-04-10 08:47:07 -07:00
epriestley
3d816e94df Rename "PhabricatorHash::digest()" to "weakDigest()"
Summary: Ref T12509. This encourages code to move away from HMAC+SHA1 by making the method name more obviously undesirable.

Test Plan: `grep`, browsed around.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12509

Differential Revision: https://secure.phabricator.com/D17632
2017-04-06 15:43:33 -07:00
epriestley
d450a08890 Support HMAC+SHA256 with automatic key generation and management
Summary:
Ref T12509. This adds support for HMAC+SHA256 (instead of HMAC+SHA1). Although HMAC+SHA1 is not currently broken in any sense, SHA1 has a well-known collision and it's good to look at moving away from HMAC+SHA1.

The new mechanism also automatically generates and stores HMAC keys.

Currently, HMAC keys largely use a per-install constant defined in `security.hmac-key`. In theory this can be changed, but in practice essentially no install changes it.

We generally (in fact, always, I think?) don't use HMAC digests in a way where it matters that this key is well-known, but it's slightly better if this key is unique per class of use cases. Principally, if use cases have unique HMAC keys they are generally less vulnerable to precomputation attacks where an attacker might generate a large number of HMAC hashes of well-known values and use them in a nefarious way. The actual threat here is probably close to nonexistent, but we can harden against it without much extra effort.

Beyond that, this isn't something users should really have to think about or bother configuring.

Test Plan:
  - Added unit tests.
  - Used `bin/files integrity` to verify, strip, and recompute hashes.
  - Tampered with a generated HMAC key, verified it invalidated hashes.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12509

Differential Revision: https://secure.phabricator.com/D17630
2017-04-06 15:42:59 -07:00
Chad Little
4d29d8e2b7 Fix filetree drag nav CSS
Summary: Fixes T11630. Not sure what the max-width fixes, but I don't see anything off on various mobile, desktop.

Test Plan: Enable filetree in differential, drag navigation all over, see normal width calculations.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T11630

Differential Revision: https://secure.phabricator.com/D17591
2017-03-31 08:37:45 -07:00
epriestley
130ebd2c42 Immediately deactivate remarkup autocomplete if there's no query
Summary:
Fixes T12479. If you end a line with a character like ":" in a context which can trigger autocomplete (e.g., `.:`), then try to make a newline, we swallow the keystroke.

Instead, allow the keystroke through if the user hasn't typed anything else yet.

Test Plan:
  - Autocompleted emoji and users normally.
  - In an empty textarea, typed `.:<return>`, got a newline instead of a swallowed keystroke.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12479

Differential Revision: https://secure.phabricator.com/D17583
2017-03-30 11:29:54 -07:00
epriestley
add1038109 Don't summon the emoji autocompleter for ":3"
Summary:
Fixes T12460. Also ":)", ":(", ":/", and oldschool ":-)" variants.

Not included are variants with actual letters (`:D`, `:O`, `:P`) and obscure variants (`:^)`, `:*)`).

Test Plan: Typed `:3` (no emoji summoned). Typed `:dog3` (emoji summoned). Typed `@3` (user autocomplete summoned).

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12460

Differential Revision: https://secure.phabricator.com/D17577
2017-03-28 15:50:56 -07:00
epriestley
aea46e55da Fix an issue where "Request Review" of a fully-accepted revision would transition to "Accepted"
Summary:
Ref T10967. This is explained in more detail in T10967#217125

When an author does "Request Review" on an accepted revision, void (in the sense of "cancel out", like a bank check) any "accepted" reviewers on the current diff.

Test Plan:
  - Create a revision with author A and reviewer B.
  - Accept as B.
  - "Request Review" as A.
  - (With sticky accepts enabled.)
  - Before patch: revision swithced back to "accepted".
  - After patch: the earlier review is "voided" by te "Request Review", and the revision switches to "Review Requested".

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10967

Differential Revision: https://secure.phabricator.com/D17566
2017-03-28 11:50:15 -07:00
Chad Little
9e2ab4f80e Scope syntax css rules to direct descendants only in diffs
Summary: Fixes T11641. We're overbroad here (and this may need more scoping?) but this seems to resolve the immediate issue.

Test Plan: Upload a few diffs and ask disabled accounts to comment on them inline.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T11641

Differential Revision: https://secure.phabricator.com/D17565
2017-03-27 10:25:37 -07:00
Chad Little
76404c5fdb Cleaner fullscreen / preview states for Remarkup bar
Summary: General CSS and usability touchup of the Remarkup bar states for fullscreen and preview. Larger fonts, more spacing, some hint of the underlying page. Disable buttons that can't be used in preview mode.

Test Plan:
Formal test coming with mobile, browsers. This is a kick the tires upload.

{F4283448}

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D17563
2017-03-27 09:19:23 -07:00
Mukunda Modell
e41c25de50 Support multiple fulltext search clusters with 'cluster.search' config
Summary:
The goal is to make fulltext search back-ends more extensible, configurable and robust.

When this is finished it will be possible to have multiple search storage back-ends and
potentially multiple instances of each.

Individual instances can be configured with roles such as 'read', 'write' which control
which hosts will receive writes to the index and which hosts will respond to queries.

These two roles make it possible to have any combination of:

* read-only
* write-only
* read-write
* disabled

This 'roles' mechanism is extensible to add new roles should that be needed in the future.

In addition to supporting multiple elasticsearch and mysql search instances, this refactors
the connection health monitoring infrastructure from PhabricatorDatabaseHealthRecord and
utilizes the same system for monitoring the health of elasticsearch nodes. This will
allow Wikimedia's phabricator to be redundant across data centers (mysql already is,
elasticsearch should be as well).

The real-world use-case I have in mind here is writing to two indexes (two elasticsearch clusters
in different data centers) but reading from only one. Then toggling the 'read' property when
we want to migrate to the other data center (and when we migrate from elasticsearch 2.x to 5.x)

Hopefully this is useful in the upstream as well.

Remaining TODO:

* test cases
* documentation

Test Plan:
(WARNING) This will most likely require the elasticsearch index to be deleted and re-created due to schema changes.

Tested with elasticsearch versions 2.4 and 5.2 using the following config:

```lang=json
  "cluster.search": [
    {
      "type": "elasticsearch",
      "hosts": [
        {
          "host": "localhost",
          "roles": { "read": true, "write": true }
        }
      ],
      "port": 9200,
      "protocol": "http",
      "path": "/phabricator",
      "version": 5
    },
    {
      "type": "mysql",
      "roles": { "write": true }
     }
  ]

Also deployed the same changes to Wikimedia's production Phabricator instance without any issues whatsoever.
```

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Tags: #elasticsearch, #clusters, #wikimedia

Differential Revision: https://secure.phabricator.com/D17384
2017-03-26 08:16:47 +00:00
epriestley
aa91dc992e Record which user accepted on behalf of packages/owners reviewers
Summary:
Ref T12271. Don't do anything with this yet, but store who accepted/rejected/whatever on behalf of reviewers.

In the future, we could use this to render stuff like "Blessed Committers (accepted by epriestley)" or whatever. I don't know that this is necessarily super useful, but it's easy to track, seems likely to be useful, and would be a gigantic pain to backfill later if we decide we want it.

Test Plan: Accepted/rejected a revision, saw reviewers update appropriately.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12271

Differential Revision: https://secure.phabricator.com/D17537
2017-03-22 14:26:37 -07:00
epriestley
fab37aa4e3 When accepting revisions, allow users to accept on behalf of a subset of reviewers
Summary:
Ref T12271. Currenty, when you "Accept" a revision, you always accept it for all reviewers you have authority over.

There are some situations where communication can be more clear if users can accept as only themselves, or for only some packages, etc. T12271 discusses some of these use cases in more depth.

Instead of making "Accept" a blanket action, default it to doing what it does now but let the user uncheck reviewers.

In cases where project/package reviewers aren't in use, this doesn't change anything.

For now, "reject" still acts the old way (reject everything). We could make that use checkboxes too, but I'm not sure there's as much of a use case for it, and I generally want users who are blocking stuff to have more direct accountability in a product sense.

Test Plan:
  - Accepted normally.
  - Accepted a subset.
  - Tried to accept none.
  - Tried to accept bogus reviewers.
  - Accepted with myself not a reviewer
  - Accepted with only one reviewer (just got normal "this will be accepted" text).

{F4251255}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12271

Differential Revision: https://secure.phabricator.com/D17533
2017-03-22 14:25:04 -07:00
Chad Little
5e16e46039 Remove "Aleo" as specialized font for headers
Summary: Fixes T11865. Part of a 'clean up remarkup' pass, removing Aleo helps simplify coding, is lighter on the wire, and gives a more consistent, clean look.

Test Plan: run celerity, grep for 'aleo' and 'Aleo', test Phriction, tasks

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T11865

Differential Revision: https://secure.phabricator.com/D17535
2017-03-22 09:57:00 -07:00
epriestley
8ad5d28686 Migrate old reviewer edges to new storage
Summary:
Ref T10967. We still have double writes, so all reviewers are being written to both old and new storage. This migrates all the data in the old storage to the new storage, so both storage tables should have a complete set of data and be getting identical updates as we move forward.

After this, I can move readers over one at a time and eventually get rid of the old writes and old storage.

This loads all of the edge data into memory in a big chunk. I reached out to one install to get some more information about their data size. Ours is quite manageable and I think even large installs will probably fit into memory, but we can do this in chunks if not.

However, because the Edge table doesn't have an `id` column, we can't use either the `RawMigrationIterator` or the `MigrationIterator`, and would need to write a new `EdgeMigrationIterator`. This isn't tons of work but might not be necessary.

Test Plan: Ran the migration locally, spot-checked the results in the database for sanity and correctness.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10967

Differential Revision: https://secure.phabricator.com/D17515
2017-03-20 16:44:40 -07:00
epriestley
794b456530 Store "last comment" and "last action" diffs on reviewers
Summary:
Ref T10967. We have a "commented" state to help reviewers get a better sense of who is part of a discussion, and a "last action" state to help distinguish between "accept" and "accepted an older version", for the purposes of sticky accepts and as a UI hint.

Currently, these are first-class states, partly beacuse we were somewhat limited in what we could do with edges. However, a more flexible way to represent them is as flags separate from the primary state flag.

In the new storage, write them as separate state information: `lastActionDiffPHID` stores the Diff PHID of the last review action (accept, reject, etc). `lastCommentDiffPHID` stores the Diff PHID of the last comment (top-level or inline).

Test Plan: Applied storage changes, commented and acted on a revision. Saw appropriate state reflected in the database.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10967

Differential Revision: https://secure.phabricator.com/D17514
2017-03-20 16:44:05 -07:00
Chad Little
de4e8728b2 Add ActionIcon to PHUIListItemView, use in Dashboards
Summary: Extends PHUIListItemView to take an icon, link as an "Action Item" that displays on the right side of the menu link. Does not display on Favorites. This allows for adding edit, external, or other links (documentation?) to any menu item. Right now the secondary link is only visible when the item is selected. This feels right, but if we offer it in other ways, users may always want it visible. We could look at making it onhover.

Test Plan:
Add a bunch of random global and personal dashboards to my menu. Add a menu to Favorites, see no link. Test mobile, link works.

{F4136699}

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D17505
2017-03-16 11:32:16 -07:00
epriestley
7626ec0ce1 Correct an issue where "View Raw File" in Differential generated a file with overbroad permissions
Summary:
Via HackerOne. When you view a raw file in Differential, we currently generate a permanent file with default permissions. This may be incorrect: default permissions may be broader than the diff's permissions.

The other three methods of downloading/viewing raw files ("Download" in Diffusion and Differential, "View Raw" in Diffusion and Differential) already apply policies correctly and generate temporary files. However, this workflow was missed when other workflows were updated.

Beyond updating the workflow, delete any files we've generated in the past. This wipes the slate clean on any security issues and frees up a little disk space.

Test Plan:
  - Ran migration script, saw existing files get purged.
  - Did "View Raw File", got a new file.
  - Verified that the file was temporary and properly attached to the diff, with "NO ONE" permissions.
  - Double-checked that Diffusion already runs policy logic correctly and applies appropriate policies.
  - Double-checked that "Download Raw Diff" in Differential already runs policy logic correctly.
  - Double-chekced that "Download Raw Diff" in Diffusion already runs policy logic correctly.

Reviewers: chad

Reviewed By: chad

Differential Revision: https://secure.phabricator.com/D17504
2017-03-16 09:51:48 -07:00
Chad Little
a72d18765f Basic "Install Dashboard" workflow
Summary: Ref T12264. This allows users to install a dashboard they are viewing to their personal home menu or as a global home menu item. Has some basic ability to be extended later for maybe projects.

Test Plan:
Build a dashboard, click "Install Dashboard".

 - As user only get personal option
 - As HomeApp edit person, see both options
 - Try installation as either, with and without label set
 - Fake "global" form as user, get error
 - Don't set anything, get error

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T12264

Differential Revision: https://secure.phabricator.com/D17492
2017-03-14 14:21:56 -07:00
epriestley
251ee9b660 Add dedicated "reviewers" storage to Differential and do double writes
Summary:
Ref T10967. This is an incremental step toward removing "reviewers" back to a dedicated storage table so we can handle changes like T11050.

This adds the storage table, and starts doing double writes to it (so new or updated reviewers write to both the old edge table and the new "reviewers" table).

Then we can do a migration, swap readers over one at a time, and eventually remove the old write and old storage and then implement new features.

This change has no user-facing impact, it just causes us to write new data to two places instead of one.

This is not completely exhaustive: the Herald "Add Reviewers" action is still doing a manual EDGE transaction. I'll clean that up next and do another pass to look for anything else I missed.

This is also a bit copy/pastey for now but the logic around "RESIGN" is a little different in the two cases until T11050. I'll unify it in future changes.

Test Plan:
  - Did a no-op edit.
  - Did a no-op comment.
  - Added reviewers.
  - Removed reviewers.
  - Accepted and rejected revisions.

After all of these edits, did a `SELECT * FROM differential_reviewer` manually and saw consistent-looking rows in the database.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10967

Differential Revision: https://secure.phabricator.com/D17495
2017-03-14 11:51:51 -07:00
Chad Little
81675e3302 Stronger scoping for Conpherence chat forms
Summary: Fixes T12391. Adds better scoping to these rules to contain changes to just Conpherence.

Test Plan: Test Conpherence, Task comment, persistent chat on mobile / desktop.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T12391

Differential Revision: https://secure.phabricator.com/D17496
2017-03-13 18:03:46 -07:00
Chad Little
0cc1a5c4d5 Clean up Space select/dropdown UI alignment
Summary: This alignment is off on edit forms, from the new overflow rules. Let's re-align everything for forms too.

Test Plan:
Add 2 new spaces, go to Maniphest, edit a task, see proper alignment of [Space] [Policy] view dropdowns. Check mobile alignment and Safari/FF.

{F3942187}

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D17491
2017-03-11 09:43:09 -08:00
Chad Little
4457c3866b Fix project hovercard tag alignment
Summary: Fix tag alignment on project cards when there are multiple tags. Also fixes T12381.

Test Plan: Review a project and people hovercard in sandbox, ensure multiple tags look as expected.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T12381

Differential Revision: https://secure.phabricator.com/D17488
2017-03-11 09:41:39 -08:00
Chad Little
3277d8b8c8 Use text ellipsis on hovercard tags
Summary: Fixes T12326. Users can make really log tag titles, this forces ellipsis if it is too long.

Test Plan: Write a super long tag, see ellipisis. Test a small tag, see normal layout.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T12326

Differential Revision: https://secure.phabricator.com/D17486
2017-03-09 11:08:57 -08:00
Chad Little
4c7d464f8b Fix firefox submenu arrows on action lists
Summary: Fixes T12370. Moves this to be absolutely positions so the float doesn't mess up text-overflow layouts.

Test Plan: Chrome, Safari, Firefox, mobile and desktop layouts of Maniphest submenus.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T12370

Differential Revision: https://secure.phabricator.com/D17484
2017-03-09 07:25:13 -08:00
Chad Little
2fed5b6925 Fourth fix for the magical world of crumbs and text-overflow
Summary: The Safari hack in place casued a truncation issue in Firefox, so that hack is now gone. Instead the bug appears to be the creative inclusion of "space". In fiddling with this adding one space inside the span and one space outside the span seems to resolve all cases.

Test Plan: Chrome, Safari, Firefox. Test "hector" and copy paste of a Task ID.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D17483
2017-03-09 07:23:41 -08:00
Chad Little
2ebdf2e080 Remove server side limit on policy control names
Summary: Fixes T12367. CSS here already truncates (or should have been) and is generally more effective. Remove the unneeded server side truncation. Any other UI place these render?

Test Plan: Set Policy to a group name of "Stanford University: Alumni Association and Friends" and see better truncation.

Reviewers: epriestley, eliaspro

Reviewed By: epriestley, eliaspro

Subscribers: eliaspro, Korvin

Maniphest Tasks: T12367

Differential Revision: https://secure.phabricator.com/D17479
2017-03-07 16:37:57 -08:00
Chad Little
683b0221c3 Fix long project names in policy controls
Summary: Fixes T12366. Adds a text-overflow to policy buttons.

Test Plan: Set a view policy to MMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMM

Reviewers: epriestley, eliaspro

Reviewed By: epriestley, eliaspro

Subscribers: eliaspro, Korvin

Maniphest Tasks: T12366

Differential Revision: https://secure.phabricator.com/D17478
2017-03-07 16:36:41 -08:00
Chad Little
0d12bbb903 Set a max-width on crumbs just in case
Summary: Ref D17361. This sets a reasonable width on crumbs just in case a title is super long. Also fixes a weird Safari issue.

Test Plan:
Set a username to "hector" and check Safari. Create a badge named "MMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMM" and test length.

{F3771744}

{F3771747}

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D17472
2017-03-06 10:47:17 -08:00
Chad Little
cd6b284678 Highlight strong tag in document headers
Summary: Fixes T12357. Adds some color to highlighted text in headers.

Test Plan: == Header with **strong** copy ==

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T12357

Differential Revision: https://secure.phabricator.com/D17469
2017-03-06 09:28:50 -08:00
Chad Little
f2e013c2e9 Prep user table for default images
Summary: Ref T10319. Adds in database columns for upcoming default generated avatar support.

Test Plan: Ran storage upgrade, log into local site to verify it didn't blow up.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T10319

Differential Revision: https://secure.phabricator.com/D17459
2017-03-04 08:18:07 -08:00
Chad Little
f095a81b00 Allow custom image generation when choosing a profile image
Summary: Ref T10319. This swaps the default in the Picture Chooser to allow picking of the custom unique avatar. We're currently going with 100k unique possibilities. The logic roughly hashes a user name and picks an image pack, color, and border. Based on that, we select the first character of their username, or fall back to Psyduck if not [a-z][0-9].

Test Plan:
Set the following usernames from ProfilePicture as a test: chad, epriestley, sally, 007, _cat_, -doggie-.

{F3453979}

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T10319

Differential Revision: https://secure.phabricator.com/D17430
2017-03-03 20:21:31 -08:00
Chad Little
87304e360f Remove dashboard footer
Summary: Doesn't seem popular, will rethink dashboard editing again in the future at some point.

Test Plan: Review a dashboard, edit, install.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D17450
2017-03-02 06:29:39 -08:00
epriestley
6f7bb8c91a On workboards, provide all of the supported "create task" forms in the dropdown
Summary:
Ref T12314. Ref T6064. Ref T11580. If an install defines several different task create forms (like "Create Plant" and "Create Animal"), allow any of them to be created directly onto a workboard column.

This is just a general consistency improvement that makes Custom Forms and Workboards work together a bit better. We might do something fancier eventually for T6064 (which wants fewer clicks) and/or T11580 (which wants per-workboard control over forms or defaults).

Test Plan:
  - Created several different types of tasks directly onto a workboard.
  - Faked just one create form, saw the UI unchanged (except that it respects any renaming).

{F3492928}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12314, T11580, T6064

Differential Revision: https://secure.phabricator.com/D17446
2017-03-02 04:24:40 -08:00
epriestley
dc7ecf5875 Add "subtype" storage to Maniphest tasks
Summary: Ref T12314. Provides a field on tasks for storing subtypes. Does nothing interesting yet.

Test Plan:
  - Ran storage upgrade.
  - Created some tasks.
  - Looked in the database.
  - Used Conduit to query some tasks.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12314

Differential Revision: https://secure.phabricator.com/D17441
2017-03-02 04:17:47 -08:00
epriestley
91ef237290 Add a "subtype" field to EditEngine forms
Summary:
Ref T12314. This adds storage so EditEngine forms can later be marked as edit fields for particular types of objects (like an "animal edit form" vs a "plant edit form").

We'll take you to the right edit form when you click "Edit" by selecting among forms with the same subtype as the task.

This doesn't do anything very interesting on its own.

Test Plan:
  - Ran `bin/storage upgrade`.
  - Verified database got the field with proper values.
  - Created a new form, checked the database.
  - Ran unit tests.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12314

Differential Revision: https://secure.phabricator.com/D17439
2017-03-02 04:16:27 -08:00
Chad Little
05377bea19 Add an avatar builtin file generator
Summary: Ref T10319. This builds out a reasonably decent avatar generator. 256 colors x 74 images x 2 borders, 38k options. Not completely sure though how names disburse though, so likely half that number. I can add lowercase lettering to double the footprint if needed though.

Test Plan:
UIExamples. Color generator here: http://tools.medialab.sciences-po.fr/iwanthue/

{F3416622}

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T10319

Differential Revision: https://secure.phabricator.com/D17418
2017-02-27 11:09:30 -08:00
Chad Little
be785d1e8a Add some pre-built alphanumeric characters
Summary: Ref T10319. Builds out some decent looking letters and numbers.

Test Plan: Review alignment in Photoshop.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T10319

Differential Revision: https://secure.phabricator.com/D17417
2017-02-26 11:39:05 -08:00
Chad Little
44b307f28d Add some higher resolution default user images
Summary: Looks nicer on profiles, cards. Added some additional colors.

Test Plan: change my avatar a few times

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: avivey, Korvin

Differential Revision: https://secure.phabricator.com/D17416
2017-02-26 09:56:21 -08:00
Chad Little
730d88c414 Re-center timeline badges
Summary: This layout inadvertantly changed, so make timeline badges always center.

Test Plan: Review in sandbox.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T12270

Differential Revision: https://secure.phabricator.com/D17411
2017-02-24 14:04:13 -08:00
Chad Little
80cccebca2 Build a Badges page for Profiles
Summary: Ref T12270. Moves badges into their own page and menu item. Capable of displaying hundreds of useful tokens of appreciation and dedication.

Test Plan:
Test blank state, mobile, awards badges.

{F3284139}

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T12270

Differential Revision: https://secure.phabricator.com/D17410
2017-02-24 13:15:42 -08:00
epriestley
84aff44bcd Add a "Red/Green Colorblind" accessibility mode, make all web UIs and email respect it
Summary:
Fixes T12172. Fixes T12060. This allows runtime code building CSS for mail to read CSS variables, then makes all the code do that.

It reverts the non-colorblind red/green to the colors in use before T12060, which seem better for non-colorblind users since no one really complained?

Test Plan:
  - Viewed code diffs in Web UI.
  - Viewed prose diffs in Web UI.
  - Viewed code diffs in email.
  - Viewed prose diffs in email.

All modes respected the accessibility color scheme.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12172, T12060

Differential Revision: https://secure.phabricator.com/D17269
2017-02-23 10:57:39 -08:00
Chad Little
254ee82a0c Hide Conpherence durable column when printing
Summary: Fixes T12303. Hides column.

Test Plan: Print a page with `__print__=1`

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T12303

Differential Revision: https://secure.phabricator.com/D17395
2017-02-22 12:31:05 -08:00
Chad Little
aaa81b4834 Center Pager buttons
Summary: Fixes T12305. Centers the buttons, which I prefer anyways.

Test Plan: Review buttons, now centered.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T12305

Differential Revision: https://secure.phabricator.com/D17394
2017-02-22 08:34:33 -08:00
Chad Little
89ce42c15c Update people hovercard UI
Summary: Removes Badges, they felt awkward. Updates UI, larger image, better layout, more icons.

Test Plan: Review numerous layouts with fancy new search tool.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D17391
2017-02-21 14:41:10 -08:00
epriestley
ab9c1b73b5 Fix bad JS rendering in "Allow Desktop Notifications" workflow
Summary:
See downstream <https://phabricator.kde.org/T5404>. This code was doing some `.firstChild` shenanigans which didn't survive some UI refactoring.

This whole UI is a little iffy but just unbreak it for now.

Test Plan: Allowed and rejected desktop notifications, got largely reasonable UI rendering.

Reviewers: chad

Reviewed By: chad

Differential Revision: https://secure.phabricator.com/D17388
2017-02-20 12:55:34 -08:00
epriestley
c0cfd4e9aa Revert "Restrict all crumbs to 240 max width" for this release
Summary:
Ref T12287. See D17361. That fixed a mostly-theoretical bug with crumbs named things like "MMMMMMMMMMMMMMMMMMMM", but caused a less-theoretical buggy side effect in Safari.

For now, just keep the "MMMMMMMM" crumbs around since that's the easiest/least-bad/safest fix prior to the release cut. We can fix this more broadly when we have more time to look at it.

Test Plan: Looked at profiles, saw the entire name for "hector" and the too-long-crumb for "MMMMM".

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12287

Differential Revision: https://secure.phabricator.com/D17382
2017-02-17 15:43:47 -08:00
Chad Little
e0cd3062d5 Restrict all crumbs to 240 max width
Summary: Ref T12270. Any project, badge, dashboard, etc, that uses names in crumbs can over generate a long title. Restrict to a sane but generous width.

Test Plan: Make a project with a really long name, test various crumb layouts, boards, tasks, desktop, mobile.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T12270

Differential Revision: https://secure.phabricator.com/D17361
2017-02-15 13:06:11 -08:00
Chad Little
37ac0ada17 Restrict movable panels to non-tab panels
Summary: Fixes T12248. Adds a flag for movable panels, and only allows those to be moved. Also cleaned up some CSS rules missing once a panel was drug into a new position.

Test Plan: Try to drag a tab panel content pane, cannot. Drag normal pane, see CSS, grab and drag same panel back, CSS looks the same.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T12248

Differential Revision: https://secure.phabricator.com/D17356
2017-02-14 14:15:59 -08:00
Chad Little
128a9d13fc Fix diviner documenation hover states
Summary: Bump up the CSS scope, since we altered the normal rule for device-desktop

Test Plan: /diviner/ in sandbox

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D17352
2017-02-14 07:23:45 -08:00
Chad Little
006d74fde2 Clean up some search result alignment CSS
Summary: Aligns the search result list to the right of the input box, adds a little space, removes a double border.

Test Plan: Search on desktop, mobile, tablet breakpoints. Double check normal typeahead results.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D17345
2017-02-13 11:19:00 -08:00
Chad Little
4176bdeb5b Allow task graph task titles to go full width
Summary: Fixes T12213. Removes truncation and allows titles to be full width if needed.

Test Plan:
Chrome / Firefox / Safari on Mac, mobile and desktop widths.

{F2754679}

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T12213

Differential Revision: https://secure.phabricator.com/D17336
2017-02-10 10:58:15 -08:00
epriestley
4039f5f11b Don't try to access the Stripe object until the user submits the credit card form
Summary: Ref T12232. I can't reproduce the original issue, but this should probably fix it without side effects?

Test Plan: Added a card with Stripe, but I could do that before too.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12232

Differential Revision: https://secure.phabricator.com/D17333
2017-02-10 08:40:14 -08:00
Chad Little
d1c253de94 Touch up basic usability of Dashboards
Summary: Ref T10390. This mostly shuffles layout into "View" and keepts "Manage" around for Edit/Copy/History. This feels better to me overall. Also tweaked some spacing and color.

Test Plan:
New Dashboard, edit Dashboard, shuffle panels. Create new panels.

{F2684043}

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T10390

Differential Revision: https://secure.phabricator.com/D17326
2017-02-09 14:20:47 -08:00
Chad Little
7d0d4708ca Fix icon color on project icon chooser
Summary: These colors are also off from the icon change.

Test Plan: Project -> Manage -> Edit Picture -> Choose Icon

Reviewers: epriestley, 20after4

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D17328
2017-02-09 14:45:47 +00:00
Chad Little
1f4a89b613 More minor CSS tweaks globally
Summary: Moves profile/project to use more standard colored boxes. Reverts dashboard border colors. Ensures better High-Contrast application more consistently across these projects. Also fix T12211.

Test Plan: Home, People, Projects in High Contrast / Standard

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T12211

Differential Revision: https://secure.phabricator.com/D17321
2017-02-06 11:06:46 -08:00
Chad Little
b58e18bad7 Allow action-list-items to also use text-overflow ellipsis
Summary: These just cut off currently if the menu item name is too wide (like mobile). Add some ellipsis

Test Plan: Check long name on mobile action list menu

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D17319
2017-02-06 09:27:11 -08:00
Chad Little
b33bb3714b Add some max-width to basic-nav
Summary: Fixes T12214. Adds a max-width

Test Plan: Tested on Home, Projects, Appsearch

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T12214

Differential Revision: https://secure.phabricator.com/D17318
2017-02-06 08:58:34 -08:00
Chad Little
70135d0ca8 Lots of little minor CSS tweaks
Summary: Lots of little details, fix workboard bg colors, darken up global backgrounds just a hair, add more "widgety" look to dashboard panels, remove underline on anchors on mobile. Also Fixes T12210

Test Plan: Use lots of pages on mobile and desktop.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T12210

Differential Revision: https://secure.phabricator.com/D17315
2017-02-05 20:45:27 +00:00
Chad Little
2c691ef009 Add more color/CSS to phui-icon-selector
Summary: Fixes T12205. These got over-ridden at a global scale (correctly) and need to adjust local scopes better. Also make it more bluer.

Test Plan: Go to Edit Dashboard, and pick a new icon for a Dashboard.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T12205

Differential Revision: https://secure.phabricator.com/D17312
2017-02-04 10:13:32 -08:00
Chad Little
4686769273 Widen property list key value
Summary: Ref T12180. Seems we widened the value side, but not the key.

Test Plan: Review application property page. Set item to stacked, see non-cutoff layout.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T12180

Differential Revision: https://secure.phabricator.com/D17302
2017-02-03 08:49:26 -08:00
Chad Little
dd068a071e Add clearer saved search options in ApplicationSearch
Summary: Ref T5307, Makes these buttons a little more clear visually and verbosely. Adds white icons for blue buttons.

Test Plan: Test saving a search, viewing button changes on various form pages / uiexamples.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T5307

Differential Revision: https://secure.phabricator.com/D17305
2017-02-03 08:47:49 -08:00
Chad Little
c0e3573454 Break long words on txt files in Diffusion
Summary: Fixes T12163. We don't send txt files through a pygment highlighter, so the break-word needs to also just apply to diffusion-source.

Test Plan: Test in Chrome Inspector

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T12163

Differential Revision: https://secure.phabricator.com/D17303
2017-02-02 14:11:52 -08:00
Chad Little
e980b94a2f Add a Picture Menu Item for Profiles
Summary: Just making profiles a little nicer, adds a big picture, easier mechanism for updating photos. Also larger profile pictures... need to re-thumb?

Test Plan:
View my profile, edit my picture, view a stranger, see profile. Check mobile, tablet, desktop. Check action menu on mobile.

{F2559394}

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D17295
2017-02-02 08:52:38 -08:00
Chad Little
7e3adb1257 Add FontAwesome 4.7.0
Summary: Adds most up to date version of FontAwesome

Test Plan: {icon snowflake-o}

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D17293
2017-02-01 16:34:48 -08:00
Chad Little
f4943a37ee Unselect "home" on mobile css
Summary: Ref T12174. Removed selected state CSS on mobile devices.

Test Plan: Set Chrome to responsive and check each break point.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T12174

Differential Revision: https://secure.phabricator.com/D17292
2017-02-01 16:33:42 -08:00
Chad Little
42156ed1ff Add back border on mobile search box
Summary: I removed this for cleaner UI on desktop, but still looks nice on mobile.

Test Plan: Test mobile, tablet, desktop search box.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D17291
2017-02-01 14:12:15 -08:00
Chad Little
638a9471fd Crisp CSS rules for JX.Tooltip
Summary: Ref T12174. More white and bolder. So crisp

Test Plan: Hover.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T12174

Differential Revision: https://secure.phabricator.com/D17286
2017-02-01 07:20:58 -08:00
epriestley
49b03c0353 Improve tooltip behavior for Application menu items
Summary:
Ref T12174.

  - Fix text antialiasing pop-in at the end of the animation in Safari.
  - Fix flickery animation replay when mousing inside a tooltip element, or rapidly switching between tooltip elements.
    - To accomplish this: don't play the animation if the last tip was hidden less than 500ms ago.

Test Plan:
  - Viewed a tooltip in Safari.
  - Waved cursor wildly over application items, both left-right (within an item) and up-down (between items).

Tooltips appeared stable and non-flickery in all cases.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12174

Differential Revision: https://secure.phabricator.com/D17272
2017-01-31 09:19:17 -08:00
epriestley
ad01e26af7 Redesign Home/Profile/Projects side navigation
Summary: Ref T11957. Needs some more polish, but I think everything here is square.

Test Plan: Add personal/global items to home, test mobile. Test workboards / colors.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: 20after4, rfreebern, Korvin

Maniphest Tasks: T11957

Differential Revision: https://secure.phabricator.com/D17259
2017-01-31 08:59:01 -08:00
epriestley
239b7c7f5c Fix icon spacing for adjacent build status and audit status in commit history tables
Summary: Ref T10978. These were a little too snug.

Test Plan: {F2549613}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10978

Differential Revision: https://secure.phabricator.com/D17268
2017-01-30 15:26:47 -08:00
Chad Little
01b35cdc12 Add some sort of sort to Emoji Autocomplete
Summary: Ref T12139. Adds sorting by shortname. Also I sorted everything else. No reason. It didn't help

Test Plan: `:star`

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T12139

Differential Revision: https://secure.phabricator.com/D17246
2017-01-24 20:21:06 -08:00
Chad Little
1fed61cf9d Jiggle Fonts for Windows
Summary: Moves the fonts around for better Windows fallback

Test Plan: Windows 10 Edge / Chrome

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D17245
2017-01-24 15:59:26 -08:00
Chad Little
f930fd2e00 Add an Emoji Typeahead
Summary:
This adds a more complete emoji datasource, with a typeahead and autocomplete. It works by pulling in a raw datasource from EmojiOne (I chose Unicode 8, but they have a Unicode 9 datasource as well) and transforming it for speed/need. If we build more robustness or an actual picker into the Remarkup bar, having the additional keywords, etc, might be important. When Unicode 9 support is more prevalent, we should only need to update the single file.

 Tossing up as a proof of concept on engineering direction. Also I can't quite get the autocomplete to complete.

Test Plan: Test UIExamples, Autocomplete, and TypeaheadSource

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T12139

Differential Revision: https://secure.phabricator.com/D17244
2017-01-24 13:13:10 -08:00
epriestley
61216dc641 Fix a JS error when adding a project card directly to a subproject from a workboard
Summary:
Fixes T12152. When creating a subproject task directly from a parent project, we can get column information back about the subproject column, which isn't visible. Just ignore this rather than trying to draw a card into a column which does not exist.

This flow (see test plan) is a little unintuitive since the card never appears on the workboard, but this class of things is probably roughly somewhere in T4900.

Test Plan:
  - Create a parent project A.
  - Create subproject B.
  - On the workboard for A, select "Create Task..." from the dropdown.
  - Add tag "B" in the dialog that pops up.
  - Save the task.

Note that this flow creates a task tagged only with "B", since "A" and "B" are mutually exclusive tags.

Before patch: UI freezes (JX.Mask is never removed), JS error in console.
After patch: workflow completes. Note that card is (correctly) not visible: it's in an invisible subproject column.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12152

Differential Revision: https://secure.phabricator.com/D17242
2017-01-24 07:41:32 -08:00
Chad Little
e9e4c6f6a0 Enable color emoji on Windows
Summary: Ref T12139, installs 'Segoe UI Emoji' as a standard font call for color emoji on Windows devices.

Test Plan: Review Emoji on Win 10 Chrome / Edge, Mac Chrome / Safari.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T12139

Differential Revision: https://secure.phabricator.com/D17241
2017-01-24 01:39:59 +00:00
Chad Little
58c857a681 Remove motivator panel
Summary: Removes the often funny, but never really used but will cause us bug reports someday.... cat facts.

Test Plan: Install cat facts, run storage upgrade, see no cat facts in menu.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T12126

Differential Revision: https://secure.phabricator.com/D17233
2017-01-19 14:55:19 -08:00
epriestley
f8cebdc418 Make Differential inline events actually trigger comment preview refreshes
Summary:
Earlier, I made some changes so that when you create or edit an inline, the comment at the bottom of the page updates (even though you didn't fiddle with the stacked actions inputs).

At the last second I broke them by spelling this wrong while cleaning things up, so they didn't actually work. Spell the property correctly ("showPreview", not "shouldPreview").

Also, we have some JS which rewrites "Not Visible" into "View", but it fires in an inconvenient way now and is flickery for me. Ideally this should get cleaned up slightly better eventualy, but at least make is stop doing so much flickery layout for now.

Test Plan:
  - Wrote no comment on a revision.
  - Added an inline.
  - Saw comment preview properly update immediately.

Reviewers: chad

Reviewed By: chad

Differential Revision: https://secure.phabricator.com/D17229
2017-01-19 12:32:49 -08:00
epriestley
45c3aaeb26 Attempt to make dropdown item actions more consistent
Summary:
See D17210. Currently, this handler needs to be installed on each menu that doesn't build with the default behavior.

Rather than copy-pasting it to the user menu, try to make it a default behavior. This adds a new rule: don't close the menu if the item is a dynamic item built in JS with PHUIXActionView.

This allows dynamic items to control the menu themselves, while giving static items the desired default behavior.

Test Plan:
  - Opened menus on: dashboards, user menu, timeline comments. Clicked stuff. Menus went away.
  - Other menus still seemed to work right: Diffusion, Favorites, mobile menu.

Reviewers: chad

Reviewed By: chad

Differential Revision: https://secure.phabricator.com/D17222
2017-01-18 13:14:54 -08:00
Chad Little
35f4514e3f Fancier user menu
Summary: Builds out more UI to reinforce just who you are in this world... A perfect person.

Test Plan:
Look at myself a lot.

{F2435202}

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D17224
2017-01-18 12:33:31 -08:00
epriestley
a3b067291f Add a couple more files to the core JS package
Summary: Ref T5867. We have a bit more common JS now.

Test Plan: Loaded some pages, saw fewer requests. Sanity-checked these resources against Multimeter in production.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T5867

Differential Revision: https://secure.phabricator.com/D17220
2017-01-18 08:14:41 -08:00
epriestley
a886969c48 Make documentation items in user menu update as you navigate in Quicksand
Summary: Ref T5867. I sure love Javascript.

Test Plan: Navigated between Home, Diffusion and Differential, opening the user profile menu. Saw appropraite help items.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T5867

Differential Revision: https://secure.phabricator.com/D17214
2017-01-17 15:55:52 -08:00
Chad Little
6f5dab634d Redesign header menus and search
Summary:
Still lots to fix here, punting up since I'm running into a few roadblocks.

TODO:
[] Sort Personal/Global correctly
[] Quicksand in Help Items correctly on page changes

Test Plan: Verify new menus work on desktop, tablet, mobile. Test logged in menus, logged out menus. Logging out via a menu, verify each link works as expected. Help menus get build when using an app like Maniphest, Differential. Check that search works, preferences still save.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T12107

Differential Revision: https://secure.phabricator.com/D17209
2017-01-17 12:13:06 -08:00
epriestley
a4a9485612 Hide dropdown menus when users click workflow items
Summary:
In D16157, dropdown menus got an overly-broad check for not closing when an item is clicked.

Specifically, we don't want to close the menu if the item is really opening a submenu, like "Edit Related Objects..." does on mobile.

The check for this is too broad, and also doesn't close the menu if the item has workflow.

Instead, use a narrower check.

Test Plan:
  - Menu still stays open when toggling submenus like "Edit Related Objects".
  - Menu now closes properly when using workflow items like "Edit Comment" or "Remove Comment".

Reviewers: chad

Reviewed By: chad

Differential Revision: https://secure.phabricator.com/D17210
2017-01-16 13:38:12 -08:00
Chad Little
36e53fd5d0 Remove collapsable option from ProfileMenu
Summary: Never really used this to full potential and takes up a lot of code and space. Remove option for now and make all profile nav menus small by default.

Test Plan: Review user, project, workboard. Set new menus.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T5867

Differential Revision: https://secure.phabricator.com/D17206
2017-01-13 15:03:31 -08:00
epriestley
b5722a9963 Use EditEngine stacked comments in Diffusion
Summary: Ref T10978. Ref T8739. Fixes T10446. Converts Diffusion to modern comment/preview code, like Differential.

Test Plan: {F2342933}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10978, T10446, T8739

Differential Revision: https://secure.phabricator.com/D17183
2017-01-11 14:46:48 -08:00
Chad Little
1e1a0182ca Add basic diff coloring to CelerityDefaultProcessor
Summary: Moves basic colors into the processor.

Test Plan: Review a diff in sandbox with and without change.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D17173
2017-01-10 17:54:24 -08:00
Chad Little
63af2275ca Re-darken full width diff colors
Summary: Ref T12089. This reverts back to using "bright" colors for full width changes in green and red.

Test Plan:
Review diffs, see full width colors.

{F2333933}

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T12089

Differential Revision: https://secure.phabricator.com/D17171
2017-01-10 13:25:23 -08:00
epriestley
11f3a8cfca Force 'changes' to a string in hunk migration
Summary: Fixes T12090. In obscure situations lost to the mists of time, the `changes` column could be `null`. Force a string cast so the migration finishes, even though these changesets are likely meaningless.

Test Plan:
I did a force-reapply as a sanity check:

```
$ ./bin/storage upgrade -f --apply phabricator:20161213.diff.01.hunks.php
```

That went cleanly; it would only have caught dramatic errors, but I didn't completely butcher things.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12090

Differential Revision: https://secure.phabricator.com/D17168
2017-01-10 12:28:09 -08:00
epriestley
2dfe79cfc7 When updating revisions in response to commits, reuse previously generated diffs
Summary:
Fixes T10968. In rare situations, we can generate a diff, then hit an error which causes this update to fail.

When it does, we tend to get stuck in a loop creating diffs, which can fill the database up with garbage. We saw this once in the Phacility cluster, and one instance hit it, too.

Instead: when we create a diff, keep track of which commit we generated it from. The next time through, reuse it if we already built it.

Test Plan:
  - Used `bin/differential attach-commit <commit> <revision>` to hit this code.
  - Simulated a filesystem write failure, saw the diff get reused.
  - Also did a normal update, which worked properly.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10968

Differential Revision: https://secure.phabricator.com/D17164
2017-01-09 12:13:44 -08:00
Chad Little
8a85ee7c15 Add CustomPHID to PhabricatorProfileMenuEngineConfiguration
Summary: Ref T5867, adds a customPHID field, nullable, and lets you query by it... i think? Not fully able to grok all the EditEngine stuff, but I think this is the right place for the query.

Test Plan: Not wired to anything, but pulling up project menu, editing, all still works.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T5867

Differential Revision: https://secure.phabricator.com/D17149
2017-01-07 10:49:54 -08:00
epriestley
5ff02058a4 Make JX.Tooltip more conservative about positioning
Summary:
Fixes T10687. Fixes T12064.

  - Primarily, adds a margin around the edge of the screen for the purposes of aligning the tooltip.
  - Also, tries to flip the tooltip if it can (e.g., if the tooltip normally goes east, try west first), then tries other positions exhastively.

Test Plan:
{F2309363}

{F2309364}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12064, T10687

Differential Revision: https://secure.phabricator.com/D17145
2017-01-05 16:03:25 -08:00
epriestley
103b450713 Nutritious vitamin paste.
Auditors: chad
2017-01-04 19:18:14 -08:00
Chad Little
96fbf37dcc Bring up contrast on light green / red diffs
Summary: Minor color saturation here, ideal for low quality monitors.

Test Plan:
Review new colors in various scenarios.

{F2305178}

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D17141
2017-01-04 15:18:24 -08:00
Chad Little
aa9708c5d3 Update diff highlight colors for better color blindess distinction
Summary: Tweaks the diff colors here a bit, as well as making full diffs slightly easier to read in full. Ref T12060

Test Plan:
Tested prose diffs, email prose diffs, and a regular Differential revision.

{F2304056}

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T12060

Differential Revision: https://secure.phabricator.com/D17138
2017-01-04 11:35:19 -08:00
Chad Little
489587d607 Add download link to embedded files
Summary: Ref T3612. Doesn't render correctly, need help please. Adds a download icon into the renderfilelinkview to allow easier downloads.

Test Plan: Click on link, get download, click on file, get lightbox.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T3612

Differential Revision: https://secure.phabricator.com/D16980
2017-01-03 10:50:26 -08:00
epriestley
35750b9c61 Make some Differential comment actions (like "Accept" and "Reject") conflict with one another
Summary:
Ref T11114. When a user selects "Accept", and then selects "Reject", remove the "Accept". It does not make sense to both accept and reject a revision.

For now, every one of the "actions" conflicts: accept, reject, resign, claim, close, commandeer, etc, etc. I couldn't come up with any combinations that it seems like users are reasonably likely to want to try, and we haven't received combo-action requests in the past that I can recall.

Test Plan:
  - Selected "Accept", then selected "Reject". One replaced the other.
  - Selected "Accept", then selected "Change Subscribers". Both co-existed happily.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T11114

Differential Revision: https://secure.phabricator.com/D17132
2017-01-02 13:25:12 -08:00
Chad Little
34d279abde Add responsive spacing to comment form info view
Summary: Moves spacing to responsive CSS.

Test Plan: Test mobile, desktop, and tablet breakpoints.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D17130
2017-01-02 10:43:40 -08:00
epriestley
18249b097f Make inline comment preview and submission mostly work on EditEngine
Summary: Ref T11114. This comments nearly working on EditEngine. Only significant issue I caught is that the "View" link doesn't render properly because it depends on JS which is tricky to hook up. I'll clean that up in a future diff.

Test Plan: {F2279201}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T11114

Differential Revision: https://secure.phabricator.com/D17116
2016-12-31 10:10:29 -08:00
epriestley
3c5a17ba8a Restore "Reclaim" and "Abandon" actions to Differential on EditEngine
Summary: Ref T11114. This begins restoring comment actions to Differential, but on top of EditEngine.

Test Plan: {F2263148}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T11114

Differential Revision: https://secure.phabricator.com/D17107
2016-12-31 10:06:46 -08:00
epriestley
3fedc8c299 Allow any EditEngine comment form to be pinned
Summary:
Fixes T12049. This expands "Haunted" comment panels to EditEngine, and by extension to all EditEngine applications.

Eventual goal is to remove custom commenting code in Differential and replace it with EditEngine code.

Changes from current "haunt" mode:

  - This only has one mode ("pinned"), not two ("pinned", "pinned with preview"). There's an inline preview and scroll behavior is a little better.
  - Now has a UI action button.

Slightly tricky stuff:

  - This interacts with "Fullscreen" mode since it doesn't make sense to pin a full-screen comment area.
  - This should only be available for comments, not for remarkup fields like "Description" in "Edit Task".

Test Plan:
  - Pinned/unpinned in Maniphest.
  - Pinned/fullscreened/unfullscreened/unpinned.
  - Checked that "Edit Task" doesn't allow pinning for "Description", etc.
  - Pressed "?", read about pressing "Z".
  - Pressed "Z".

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12049

Differential Revision: https://secure.phabricator.com/D17105
2016-12-29 12:49:18 -08:00
Chad Little
5bdc7dd07b Minor style updates to Phame
Summary: More obvious crumbs, remove border on images, better image spacing.

Test Plan: Build out a test blog, click on crumbs, view spacing.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D17101
2016-12-22 22:01:01 +00:00
Sébastien Santoro
178466b826 Fix 20160122.project.1.boarddefault.php database upgrade patch
Summary: Fixes T12031.

Test Plan: Upgraded a December 2015 Phabricator installation to current schema.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T12031

Differential Revision: https://secure.phabricator.com/D17094
2016-12-19 12:52:55 -08:00
Chad Little
c6bdd2c56b Add Ngram support to Dashboards / Panels
Summary: Build ngram indexs, adds search by name capability.

Test Plan: Search for a dashboard by partial name, search for a panel by partial name.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D17090
2016-12-16 12:09:06 -08:00
epriestley
3d8d98bd8d Probably (?) fix two very old project slug migrations
Summary: Fixes T12020. These callsites to `getPhrictionSlug()` were missed when that method was removed. They're very old (early 2014, late 2011).

Test Plan:
These are tricky to test because the migrations are so ancient, but `bin/storage upgrade --force --apply phabricator:20140521.projectslug.2.mig.php` gave me //plausible// results.

The other migration is so ancient that it can't apply to a modern database so I'm just kind of winging that one. We probably have essentially no installs which will ever apply it again, though.

Reviewers: chad, avivey

Reviewed By: avivey

Maniphest Tasks: T12020

Differential Revision: https://secure.phabricator.com/D17060
2016-12-14 14:14:21 -08:00
Chad Little
e077d2f7a7 Reorganize phui-object-item CSS, add drag ui
Summary: Reorgaizes the CSS here a bit, by object list style, adds in a new drag ui class, which will be used in menu ordering.

Test Plan:
Workboards, Home Apps.

{F2126266}

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D17057
2016-12-14 11:53:17 -08:00
epriestley
c67e87c809 Force hunk storage migration to the modern format
Summary:
Ref T8475. This forces installs to migrate hunks to the modern format.

We stopped writing to the legacy format a very long time ago (2+ years?) without issues.

This doesn't destroy any data. T8623 has guidance and I'll publish more changelog guidance.

Test Plan: Faked some legacy data and migrated it.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T8475

Differential Revision: https://secure.phabricator.com/D17039
2016-12-13 14:34:08 -08:00
epriestley
9d2feacfab Rename a missed Profile -> MenuItem
Summary: Didn't grep this good enough.

Test Plan: `bin/storage upgrade -f --apply ..`, got a clean apply.

Reviewers: chad

Reviewed By: chad

Differential Revision: https://secure.phabricator.com/D17046
2016-12-13 14:27:00 -08:00
Chad Little
26127b9c5f Allow Dashboards to set an icon
Summary: Allows users set an icon (for reuse on upcoming home) for their dashboard based on 16 descriminating choices.

Test Plan: Create a new dashboard, set new icon. Edit an existing dashboard, set icon.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D17042
2016-12-13 11:30:22 -08:00
Chad Little
c03a412d5c Add authorPHID to Dashboard Panels
Summary: Adds authorPHID to panels so we can default to the panels you made.

Test Plan: Run upgrade, visit manage panels, see my panels. Create a new panel. Edit a panel.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D17036
2016-12-13 10:07:16 -08:00
epriestley
776a1f3aec Rename dashboard author patches so that the backfill patch happens
after the column it is adding is created

Auditors: chad
2016-12-12 15:44:28 -08:00
Chad Little
59f3b5125d Add authorPHID to Dashboards
Summary: Adds an authorPHIDs, populates olds ones.

Test Plan: Make a new Dashboard, see that I created it.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D17022
2016-12-12 15:26:43 -08:00
Chad Little
a79e4b786e Restrict hover styles in action list to only href
Summary: We're currently applying these styles to labels, restrict them to only list elements that have an href.

Test Plan: Test a label, an anchor, and a button as an action item.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D17031
2016-12-12 09:06:46 -08:00
epriestley
8480776ccd Rename "ProfilePanelConfiguration" to "ProfileMenuItemConfiguration"
Summary:
Ref T11957. This renames the Configuration storage, transaction, query, and PHID type.

No rename on the actual menu item types yet, that's next (and should be the end of this, I think).

Test Plan:
  - Viewed projects.
  - Viewed profiles.
  - Edited a project menu.
  - Grepped for all renamed symbols, I think?

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T11957

Differential Revision: https://secure.phabricator.com/D17027
2016-12-11 11:44:22 -08:00
Chad Little
fddd467400 Fake an outline ring on buttons
Summary: Fixes T11990. Chrome/Ubuntu doesn't respect `outline: auto`, so this is the only workaround I could fine. Somewhat nicer than stock Chrome.

Test Plan: tab tab tab tab, ubuntu, chrome, windows, safari.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T11990

Differential Revision: https://secure.phabricator.com/D17023
2016-12-11 17:21:16 +00:00
Chad Little
0dfe665539 More consistent icon spacing on menus
Summary: Use one set of spacing everywhere for action menus

Test Plan: Review dropdowns, action lists.

Reviewers: epriestley, tyhtest

Reviewed By: tyhtest

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D17006
2016-12-07 13:29:51 -08:00
Chad Little
8bbed7974d Fix some form layout issues in settings
Summary: Form inputs with just text or certain selects had extra space. This uses better text centering.

Test Plan: Review forms on all Settings pages, Custom Policy, Herald.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D17005
2016-12-07 13:28:03 -08:00
Eitan Adler
0ad1dd640a Remove the Persona login method
Summary:
Persona is going to be decommed November 30th, 2016.
It is highly unlikely that anyone is currently using persona as a real
login method at this point.

Test Plan: tried locally to add auth adapter.

Reviewers: chad, #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: epriestley

Differential Revision: https://secure.phabricator.com/D16371
2016-12-05 15:57:15 -08:00
Chad Little
f0bf0419f1 Hide badges in phui-comment-panel
Summary: Ref T3612. Hides badges on the comment panel.

Test Plan: Give myself a badge, leave a comment, see no badge UI.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T3612

Differential Revision: https://secure.phabricator.com/D16979
2016-12-02 11:02:53 -08:00
Chad Little
49a20bde8e Prevent double lightbox in lightbox comments
Summary: Ref T3612, prevents lightbox from spawning from inside a lightbox.

Test Plan: Click on file lightbox, leave file comment, click file comment, get take to file page instead of another lightbox.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T3612

Differential Revision: https://secure.phabricator.com/D16978
2016-12-02 10:56:21 -08:00
Chad Little
aa6517a42b Make clicking on file icon in lightbox download file
Summary: Ref T3612, this adds a anchor around the large icon with hover state so you can download from here as well.

Test Plan: Hover over .ics file, click, get download.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T3612

Differential Revision: https://secure.phabricator.com/D16977
2016-12-02 10:21:28 -08:00
Chad Little
0db28cfc01 Wipe textarea immediately when submitting a new message in Conpherence
Summary: Fixes T11791. We do this in durable column, but not in regular Conpherence. I think this is the right place? Not sure how this will feel with high lag.

Test Plan: Submit lots of text in a Conpherence.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T11791

Differential Revision: https://secure.phabricator.com/D16969
2016-12-01 10:08:11 -08:00
Chad Little
98057091f6 Remove default margin on selects
Summary: Fixes T11935. Safari even with appearance: none still assigns margin to selects. Reset margin to 0.

Test Plan: Review selects on Safari, Chrome.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T11935

Differential Revision: https://secure.phabricator.com/D16968
2016-12-01 09:08:40 -08:00