1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-12-11 08:06:13 +01:00
Commit graph

1178 commits

Author SHA1 Message Date
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
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
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
epriestley
d03a497616 Allow any inline in the document to be queried by ID
Summary:
Ref T12616. When you "Delete" a comment from the preview, we try to delete the comment on screen too.

It may or may not be present on screen: if you just added it it's usually visible. However, you might also have hidden the file it contains or it could be on an older diff in a file which is no longer present in the current diff.

After updates in T12616, we could only find the comment if you'd previously interacted with it for some reason. Update this code to be able to find all inlines present in the document.

Test Plan:
  - Write a draft comment.
  - Reload the page.
  - DO NOT INTERACT WITH THE COMMENT!
  - Delete it from the preview.
  - After patch: Comment is deleted from the document, too.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12616

Differential Revision: https://secure.phabricator.com/D17937
2017-05-17 12:26:40 -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
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
epriestley
06c933781e Move keyboard focus reticle code to Differential
Summary:
Ref T12634. Using keyboard shortcuts in Differential currently relies on focus behavior in `KeyboardShortcutManager`.

This possibly made sense long ago, but no longer does, and leads to a whole slew of bugs where the reticle doesn't interact properly with anything else.

Move it to Differential so it can be made reasonably aware of edit operations, Quicksand navigation, etc.

This just moves the code; future diffs will actually fix bugs.

Test Plan: Used "n", "j", etc., saw the same behavior as before.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12634

Differential Revision: https://secure.phabricator.com/D17899
2017-05-16 08:20:50 -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
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
adb1d3a5be Fix autocomplete/send-on-enter interactions
Summary:
Send-on-enter and autocomplete both listen for "return" keypresses, and could race. Have autocomplete let other handlers take a shot at the action before it does.

Also, fix a case where ":)" and the suffix list (which lets you type `someone is 100% to blame here (@epriestley)` and get the results you want) interacted badly, so ":)" cancels the autocompleter like ":3" does.

Test Plan:
  - Typed "@xxx" and mashed return real fast over and over again while reloading the page. Before: sometimes handlers raced and text submitted. After: always handled by autocomplete behavior.
  - Typed ":", ")", "<return>", sent an emoticon (previously: no).

Reviewers: chad, amckinley

Reviewed By: chad

Subscribers: xxx

Differential Revision: https://secure.phabricator.com/D17794
2017-04-26 08:48:25 -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
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
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
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
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
f394fefe6f Add a very basic "Realtime" log to DarkConsole
Summary: Ref T12568. This begins building toward a more useful realtime debugging console for Leader/Aphlict/general realtime stuff.

Test Plan: {F4911521}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12568

Differential Revision: https://secure.phabricator.com/D17701
2017-04-17 15:46:31 -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
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
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
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
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
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
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
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
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
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
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
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
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
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
4b94ae15e0 Straighten out some stacked action removal behaviors
Summary: Fixes T12065. Some of the normal behavior of these actions got juggled when I made "Accept" and "Reject" conflict.

Test Plan:
  - Added "Accept".
  - Added "Reject", saw it remove "Accept".
  - Added "Change Projects".
  - Added "Assign/Claim".
  - Removed "Assign/Claim".
  - Removed "Accept".

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12065

Differential Revision: https://secure.phabricator.com/D17143
2017-01-04 19:16:58 -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
epriestley
00313094d3 Make "View" links on Differential inline comment previews work again
Summary:
Ref T11114. Recent changes broke the links to jump to inline comments from the previews because they get hooked up with JS.

Restore the linking behavior.

Test Plan: Clicked "View" on an inline comment preview, jumped to that comment.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T11114

Differential Revision: https://secure.phabricator.com/D17131
2017-01-02 13:24:02 -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
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
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
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
f844280d36 Make lightbox buttons more roundy
Summary: Ref T3612. Mobilizes the new lightbox, changes large buttons to circle icons like Conpherence.

Test Plan: Click each new button on desktop, mobile.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T3612

Differential Revision: https://secure.phabricator.com/D16961
2016-11-29 12:23:05 -08:00
Chad Little
d1838fa5ec Close lightbox when clicking on image-frame
Summary: Ref T3612. Moves the listener to the frame of the image.

Test Plan: Click on image, no close, click on grey frame, closes image. Test image and document, clicking on arrows.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T3612

Differential Revision: https://secure.phabricator.com/D16959
2016-11-29 18:35:04 +00:00
Chad Little
80570d668b Use proper file icons in lightbox
Summary: Ref T3612. Passes in file size and file icon for non-images.

Test Plan: Review a PDF and PSD in a lightbox.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T3612

Differential Revision: https://secure.phabricator.com/D16957
2016-11-28 11:30:19 -08:00
Chad Little
3edd01bc0f Remove english from lightbox javascript
Summary: Reduces to x / y for lightbox. Adds spacing. Ref T3612

Test Plan: Review spacing in sandbox.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T3612

Differential Revision: https://secure.phabricator.com/D16951
2016-11-28 08:48:14 -08:00
Chad Little
0aa3f8b86b Allow commenting on non-viewable lightbox files
Summary: Removes the viewable restriction on embedded files. Builds a basic lightbox UI for commenting.

Test Plan:
Add psd, pdf to Maniphest task, clicked on download, comment, left comment. Closed box.

{F1943726}

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T3612

Differential Revision: https://secure.phabricator.com/D16917
2016-11-22 11:29:36 -08:00
Chad Little
88a966993b Ajax commenting on lightbox images
Summary: Adds a comment box, you can put text into it, hit enter, and see it come back.

Test Plan: Put text into box, see it come back.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T3612

Differential Revision: https://secure.phabricator.com/D16907
2016-11-21 20:19:37 -08:00
Chad Little
8aeb7aa525 Show file comments on file lightboxes
Summary: Basic work in progress, but should show timeline comments for files when in lightbox mode. Looks reasonable.

Test Plan: click on images, see comments from timeline.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T3612

Differential Revision: https://secure.phabricator.com/D16896
2016-11-18 13:24:03 -08:00
epriestley
da8c378331 Allow workboard cards to be dragged again by using a larger value for infinity
Summary:
Fixes T11886. D16882 prevented workboard cards from being dragged between columns because it reduced the effective document height to almost nothing (~78px, just the menu bars).

Instead, use the larger of the document height and viewport height as "infinity".

Test Plan: Dragged columns between workboards successfully.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T11886

Differential Revision: https://secure.phabricator.com/D16885
2016-11-17 08:01:57 -08:00
epriestley
7c4c76d32a Fix a Quicksand/Lightbox contention issue
Summary:
Fixes T11785. Lightbox calls `JX.Stratcom.pass()` to let other handlers react, but should not. At least today, we never put, e.g., links inside a lightbox.

This code appears in the original commit so it was probably just copy/pasted from somewhere and I missed it in review.

(Or there's some edge case I'm not thinking of and we'll figure it out soon enough.)

Additionally, blacklist `/file/data/` from Quicksand naviagtion: Quicksand should never fetch these URIs.

Test Plan:
  - Disabled `security.alternate-file-domain`.
  - Enabled Quicksand ("Persistent Chat").
  - Clicked an image thumbnail on a task.
    - Repeated that until things flipped out a bit.
    - After the patch: no issues.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T11785

Differential Revision: https://secure.phabricator.com/D16884
2016-11-16 17:10:56 -08:00
epriestley
f91269a4a8 On tablet/mobile, open the datepicker control in the center of the screen and mask the background
Summary:
Ref T11816. We currently try to expand the picker control from the little calendar icon. This works alright on desktop, but not great on tablet/mobile.

On tablet/mobile, center the control on screen instead. Also, mask the background since it can get a bit busy because we can't really control what's under the control anymore.

Finally, move the little popup thing around if the user resizes the window.

Test Plan: {F1922773}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T11816

Differential Revision: https://secure.phabricator.com/D16866
2016-11-15 10:19:49 -08:00
Chad Little
ebecbeed42 Rebuild lightbox styles
Summary: Simpler, lighter background and UI for lightbox. Removes images, uses font icons, buttons.

Test Plan:
Lots of lightboxing.

{F1917111}

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D16844
2016-11-12 15:37:16 +00:00
Chad Little
d78802f3ab Redesign Comment Box
Summary: Redesign the action comment box for better use in two column, mobile, nuance.

Test Plan: Test in mobile / desktop / tablet, adding and removing actions. Actionless comment boxes, etc.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: cspeckmim, Korvin

Differential Revision: https://secure.phabricator.com/D16811
2016-11-09 10:36:25 -08:00
epriestley
9a10413dbc Improve typeahead behavior with mixed-case matches
Summary:
Ref T8510. We had two issues with mixed-case result sorting, like typing `@joe` to match user `Joe`.

  - The fallback sort was not normalized properly, so "J" could sort after "j". Instead, normalize values for sorting.
  - The `prefix_hits` and older `priority_hits` mechanisms were competing destructively. The `prefix_hits` mechanism completely replaces the `priority_hits` mechanism. Instead, use only the `prefix_hits` mechanism.

Test Plan:
  - Copied results for "joe" from WMF.
  - Hard-coded the controller to return them.
  - Searched for `@joe`.
  - After patches, first hit is user "Joe".

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T8510

Differential Revision: https://secure.phabricator.com/D16826
2016-11-08 11:50:15 -08:00
epriestley
3f2f81a1c8 Remove obsolete Calendar event date storage fields
Summary: Ref T11809. These have been replaced with more flexible storage that accommodates a wider range of behaviors, including those in the ICS format and RRULEs.

Test Plan:
  - Ran migration.
  - Viewed, created, edited events.
  - Grepped for all removed names/symbols.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T11809

Differential Revision: https://secure.phabricator.com/D16789
2016-11-02 09:49:08 -07:00
epriestley
f7b0c09ac4 Make the "All Day Event" control use a checkbox instead of a dropdown
Summary:
This feels a little cleaner:

  - Clean up transaction log a bit.
  - Use a checkbox instead of a two-option dropdown.

This is a little messy because the browser doesn't send anything if the user submits a form with an un-clicked checkbox.

We now send a dummy value ("Hey, there's definitely a checkbox in this form!") so the server can figure out what to do.

Test Plan:
  - Edited all-dayness of an event.
  - Viewed transaction log.

Reviewers: chad

Reviewed By: chad

Differential Revision: https://secure.phabricator.com/D16776
2016-10-31 14:18:59 -07:00
Chad Little
1fdb8ba112 JX.Favicon for Conpherence
Summary: I think maybe these should be more separate from JX.Title, but seems to work ok. May build new favicons just for messages though. Proof of concept UI.

Test Plan: Send message on one browser, see red icon in other browser. Click on menu, count and favicon switch back to normal.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D16734
2016-10-20 12:17:50 -07:00
Chad Little
89f0015ae6 Don't show participants in Conpherence left open on mobile
Summary: Fixes T11764. Moves rendering of the column to client-side, which can skip if it detects we're on mobile.

Test Plan: Open column on desktop, switch to mobile, don't see column. Toggle column on mobile on and off. Switch back to desktop.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T11764

Differential Revision: https://secure.phabricator.com/D16725
2016-10-19 08:29:45 -07:00
epriestley
f9f25c1e4d Allow users to drop .ics files on calendar views to import them
Summary:
Ref T10747. When a user drops a ".ics" file or a bunch of ".ics" files into a calendar view, import the events.

(Possibly we should just do this if you drop ".ics" files into any application, but we can look at that later.)

Test Plan: Dropped some .ics files into calendar views, got imports.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10747

Differential Revision: https://secure.phabricator.com/D16722
2016-10-18 15:26:44 -07:00
Chad Little
ac8e11359d Remove 'full-display' setting from Conpherence, spruce up search results
Summary: This removes 'full-display', 'minimal-display' from Conpherence, which I recall was because we had 2 UIs for column and regular chat. I'm also tossing in slightly nicer search results, with a link to the actual message and the full date shown for context.

Test Plan: Post a message in mobile, tablet, full conpherence, and in durable column. Clean up UI in durable column. Do a search in Full UI, click on result date, get taken to the message... usually. My test data is a little wonky, but I think this works most of the time.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D16710
2016-10-17 15:53:55 -07:00
Chad Little
10e464142d Focus search or pontificate in Conpherence when toggled
Summary: This focuses the search field when the user opens search, and then the textarea for pontificate if the search box is closed.

Test Plan: Open/Close/Open/Close

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D16709
2016-10-15 13:59:30 +00:00
Chad Little
52dd354dad Fix Conphernce sometimes searching wrong room
Summary: I passed this in as a config, but need to parse it live when threads change, otherwise the wrong room could be searched.

Test Plan: Search in one room, click a second, search again, see correct results.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D16707
2016-10-14 09:38:23 -07:00
Chad Little
636eaf231e Only show loading animation on thread change in Conpherence
Summary: Fixes the send on enter flash, only uses the `Threads` loading animation on changing threads, not sending a message.

Test Plan: Change threads, post a message.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D16705
2016-10-13 19:55:51 -07:00
Chad Little
508a2a1498 Basic Conpherence Search in Thread
Summary: Adds a search bar toggle and results for searching inside a Conpherence Room. The UI of the results itself are not styled yet, and will follow up with another diff.

Test Plan: Go to Conpherence, search for "asdf", get lots of results. Search for nothing, get no change, search for something fictitious, get no threads found (will follow up with search result UI).

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D16697
2016-10-13 12:29:50 -07:00
Chad Little
d3487b6371 Remove unused drag and drop Conpherence code
Summary: Ref T11730. Removes unused code since this is now it's own page.

Test Plan: rebuild maps, grep for javelin code, classnames

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T11730

Differential Revision: https://secure.phabricator.com/D16700
2016-10-13 06:22:10 -07:00
Chad Little
524906a439 Remove Crop Image from Conpherence Edit UI
Summary: Ref T11730. Removes the front end crop feature. Will follow up with proper removal, but this seems broken outright.

Test Plan:
Edit a room, don't seen "Crop" feature. Upload new photo, works fine.

- grep for `ConpherencePicCropControl`
- grep for `aphront-crop`

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T11730

Differential Revision: https://secure.phabricator.com/D16665
2016-10-04 16:25:52 -07:00
Chad Little
55a56c09e7 Always reset scroll after durable column is maximized
Summary: When the durable column is re-opened, scroll user to latest message. Though later we should scroll them to the last read.

Test Plan: Minimize window, reload page, pop up window, see proper scroll position.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D16649
2016-10-02 20:32:43 -07:00
Chad Little
a591b86d91 Add an icon to aphlict connection status
Summary: A bit better styling, this adds an indication icon for if you're connected or not (and later, away, etc).

Test Plan: Test in Notifications menu, Conpherence full, Durable Column.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D16647
2016-10-02 08:17:21 -07:00
Chad Little
45af6d7c0e Set body classes via Quicksand config
Summary: Sends and stores additional body classes at the page level. Removes old ones, sets new ones.

Test Plan: home -> application search -> colored workboard -> config -> home with persistent chat open and minimized.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D16646
2016-10-01 16:36:28 -07:00
Chad Little
2f2126ecce Add 'Persist Chat' option in Conpherence notification menu
Summary: This exposes the chat window to a larger audience beside people who accidentaly hit `\`.

Test Plan:
Lots of clicks and reloads.

{F1856043}

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D16643
2016-10-01 11:36:05 -07:00
Chad Little
6d82fcc6d7 Allow Durable Column to be minimized
Summary: Add ability to minimize durable column

Test Plan:
Shrink and Grow, reload page, see stickyness...

{F1855051}

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D16638
2016-09-30 20:24:18 +00:00
Chad Little
95d1749566 Convert Durable Column to popup chat in footer
Summary: This feels pretty reasonable with little effort, and I think I'd use it more than the full column.

Test Plan:
Chat a lot on various pages.... still some quicksand quirks around various pages.

{F1853487}

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: scp, Korvin

Differential Revision: https://secure.phabricator.com/D16627
2016-09-29 10:08:14 -07:00
Chad Little
700666ae0a Make Conpherence Pontificate Send-on-Enter
Summary: Fixes T11623. Enables send-on-enter and shift-enter for linebreaks, per durable column. Also cleaned up UI for Joining Room or Logging In.

Test Plan: See room I can join, click Join Room. Leave Room, Log out, visit room with login prompt. Login, Join Room again.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T11623

Differential Revision: https://secure.phabricator.com/D16595
2016-09-27 19:54:07 -07:00
epriestley
f2bb9bc061 When summing points on a workboard, display sum with same precision as most-precise value
Summary:
Fixes T11703. This mostly avoids rounding errors.

If point values include "0.001", we also get three digits of precision: 1.000.

Maybe useful if your point field is called "bitcoins" or something.

Test Plan:
Before:

{F1851404}

After:

{F1851405}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T11703

Differential Revision: https://secure.phabricator.com/D16601
2016-09-27 09:36:46 -07:00
Chad Little
f6023d17da Rebuild Conpherence Participants Pane
Summary: Mostly quality of life in renames and moving everything to the view class. Minor CSS tweaks. Fix room handles getting added when adding a new user.

Test Plan: Add to room, remove from room.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D16560
2016-09-15 13:21:31 -07:00
Chad Little
c1c5fbce21 Rebuild Conpherence
Summary:
Minor rebuild / redesign of Conpherence. Most of this is new UX and tossing out things like widgets, device fallbacks. I expect some of the UI to get more polished after next pass, but most everything here is in place.

 - Removed "Widgets", now just a single Participants pane
 - Added "Topic"
 - New header
 - Settings, Edit are action icons
 - Removed a lot of JS
 - Simplified CSS as much as I could

Test Plan:
Desktop, Tablet, Mobile. Adding and removing people. Setting new topics, new rooms.

{F1828662}

{F1828669}

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D16550
2016-09-14 18:51:32 -07:00
Chad Little
ff64c4e02b Add a toggle, sticky pref, for Conpherence Widget Pane
Summary: This adds a "column" icon into crumbs, like in workboards, for expanding or hiding the "Widget Pane". This is per user sticky and defaults to off.

Test Plan: View a Conpherence Room, see no widgets by default. Toggle it on, see widget. Reload page, see widget stick. Verify mobile, tablets ignore hiding.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T10364

Differential Revision: https://secure.phabricator.com/D16533
2016-09-12 23:20:21 +00:00
Chad Little
d25a9530a7 Move search, new room into thread column in Conpherence
Summary:
Moves search and new room out of crumb bar and into the thread column. This opens up space to add "Edit Room" into the crumbs area as an action link. Also removed 'widget-edit' pathways and javascript.

Fixes T8972

Test Plan:
Search, New, Edit rooms. Click different rooms, see correct URL each time.

{F1813226}

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T8972

Differential Revision: https://secure.phabricator.com/D16543
2016-09-12 22:19:24 +00:00
epriestley
74d5ae7bec Clean up some log spam caused by races in VersionedDraft
Summary:
Two minor issues that I caught in the log while fixing Phame permissions:

  - We had a JS bug which would cause us to immediately generate two comment previews at the exact same time -- one for loading the page, and one for "switching to desktop". Instead, only generate the "switch to desktop" preview if we really switched to desktop from a different device layout.
  - These two requests could end up reading/writing the VersionedDraft table at exactly the same time fairly often (e.g., after a comment submission, the page would load, send two preview requests at exactly the same time, and they'd race fairly reliably for me locally). If we do race, recover from the race.

Test Plan:
Submitted some Phame comments.

  - No more error log errors about VersionedDraft keys.
  - Saw only one preview request when loading the page instead of two.

Here's the specific stack trace I caught:

```
[Mon Sep 05 12:15:33.639930 2016] [:error] [pid 50608] [client 127.0.0.1:55278] [2016-09-05 14:15:33] EXCEPTION: (AphrontDuplicateKeyQueryException) #1062: Duplicate entry 'PHID-POST-fknnpzjnsdgc3rqobhst-PHID-USER-pr5rjpuilpfserepsd2k-13' for key 'key_object' at [<phutil>/src/aphront/storage/connection/mysql/AphrontBaseMySQLDatabaseConnection.php:314]
[Mon Sep 05 12:15:33.640801 2016] [:error] [pid 50608] [client 127.0.0.1:55278] arcanist(head=master, ref.master=9e82ef979e81), corgi(head=master, ref.master=5b9171222bc9), instances(head=stable, ref.master=485bc8128198, ref.stable=2983bc917601), ledger(head=master, ref.master=4da4a24b8779), libcore(), phabricator(head=phame2, ref.master=4b6da9735ba7, ref.phame2=4b6da9735ba7), phutil(head=stable, ref.master=97f05269fdb1, ref.stable=c14343ee620e), services(head=stable, ref.master=1fcb5cdb7582, ref.stable=2d8088a5b4b3)
[Mon Sep 05 12:15:33.640815 2016] [:error] [pid 50608] [client 127.0.0.1:55278]   #0 <#2> AphrontBaseMySQLDatabaseConnection::throwCommonException(integer, string) called at [<phutil>/src/aphront/storage/connection/mysql/AphrontBaseMySQLDatabaseConnection.php:348]
[Mon Sep 05 12:15:33.640830 2016] [:error] [pid 50608] [client 127.0.0.1:55278]   #1 <#2> AphrontBaseMySQLDatabaseConnection::throwQueryCodeException(integer, string) called at [<phutil>/src/aphront/storage/connection/mysql/AphrontBaseMySQLDatabaseConnection.php:289]
[Mon Sep 05 12:15:33.640833 2016] [:error] [pid 50608] [client 127.0.0.1:55278]   #2 <#2> AphrontBaseMySQLDatabaseConnection::throwQueryException(mysqli) called at [<phutil>/src/aphront/storage/connection/mysql/AphrontBaseMySQLDatabaseConnection.php:185]
[Mon Sep 05 12:15:33.640836 2016] [:error] [pid 50608] [client 127.0.0.1:55278]   #3 <#2> AphrontBaseMySQLDatabaseConnection::executeRawQuery(string) called at [<phutil>/src/xsprintf/queryfx.php:8]
[Mon Sep 05 12:15:33.640839 2016] [:error] [pid 50608] [client 127.0.0.1:55278]   #4 <#2> queryfx(AphrontMySQLiDatabaseConnection, string, string, string, array, string)
[Mon Sep 05 12:15:33.640841 2016] [:error] [pid 50608] [client 127.0.0.1:55278]   #5 <#2> call_user_func_array(string, array) called at [<phutil>/src/aphront/storage/connection/AphrontDatabaseConnection.php:42]
[Mon Sep 05 12:15:33.640844 2016] [:error] [pid 50608] [client 127.0.0.1:55278]   #6 <#2> AphrontDatabaseConnection::query(string, string, string, array, string) called at [<phabricator>/src/infrastructure/storage/lisk/LiskDAO.php:1261]
[Mon Sep 05 12:15:33.640846 2016] [:error] [pid 50608] [client 127.0.0.1:55278]   #7 <#2> LiskDAO::insertRecordIntoDatabase(string) called at [<phabricator>/src/infrastructure/storage/lisk/LiskDAO.php:1106]
[Mon Sep 05 12:15:33.640849 2016] [:error] [pid 50608] [client 127.0.0.1:55278]   #8 <#2> LiskDAO::insert() called at [<phabricator>/src/infrastructure/storage/lisk/LiskDAO.php:1075]
[Mon Sep 05 12:15:33.640851 2016] [:error] [pid 50608] [client 127.0.0.1:55278]   #9 <#2> LiskDAO::save() called at [<phabricator>/src/applications/draft/storage/PhabricatorVersionedDraft.php:65]
[Mon Sep 05 12:15:33.640854 2016] [:error] [pid 50608] [client 127.0.0.1:55278]   #10 <#2> PhabricatorVersionedDraft::loadOrCreateDraft(string, string, integer) called at [<phabricator>/src/applications/transactions/editengine/PhabricatorEditEngine.php:1669]
[Mon Sep 05 12:15:33.640857 2016] [:error] [pid 50608] [client 127.0.0.1:55278]   #11 <#2> PhabricatorEditEngine::buildCommentResponse(PhamePost) called at [<phabricator>/src/applications/transactions/editengine/PhabricatorEditEngine.php:894]
[Mon Sep 05 12:15:33.640859 2016] [:error] [pid 50608] [client 127.0.0.1:55278]   #12 <#2> PhabricatorEditEngine::buildResponse() called at [<phabricator>/src/applications/phame/controller/post/PhamePostEditController.php:60]
[Mon Sep 05 12:15:33.640862 2016] [:error] [pid 50608] [client 127.0.0.1:55278]   #13 <#2> PhamePostEditController::handleRequest(AphrontRequest) called at [<phabricator>/src/aphront/configuration/AphrontApplicationConfiguration.php:237]
[Mon Sep 05 12:15:33.640865 2016] [:error] [pid 50608] [client 127.0.0.1:55278]   #14 phlog(AphrontDuplicateKeyQueryException) called at [<phabricator>/src/aphront/handler/PhabricatorAjaxRequestExceptionHandler.php:27]
[Mon Sep 05 12:15:33.640868 2016] [:error] [pid 50608] [client 127.0.0.1:55278]   #15 PhabricatorAjaxRequestExceptionHandler::handleRequestException(AphrontRequest, AphrontDuplicateKeyQueryException) called at [<phabricator>/src/aphront/configuration/AphrontApplicationConfiguration.php:644]
[Mon Sep 05 12:15:33.640870 2016] [:error] [pid 50608] [client 127.0.0.1:55278]   #16 AphrontApplicationConfiguration::handleException(AphrontDuplicateKeyQueryException) called at [<phabricator>/src/aphront/configuration/AphrontApplicationConfiguration.php:242]
[Mon Sep 05 12:15:33.640873 2016] [:error] [pid 50608] [client 127.0.0.1:55278]   #17 AphrontApplicationConfiguration::processRequest(AphrontRequest, PhutilDeferredLog, AphrontPHPHTTPSink, MultimeterControl) called at [<phabricator>/src/aphront/configuration/AphrontApplicationConfiguration.php:149]
[Mon Sep 05 12:15:33.640879 2016] [:error] [pid 50608] [client 127.0.0.1:55278]   #18 AphrontApplicationConfiguration::runHTTPRequest(AphrontPHPHTTPSink) called at [<phabricator>/webroot/index.php:17]
```

Reviewers: chad

Reviewed By: chad

Differential Revision: https://secure.phabricator.com/D16494
2016-09-05 13:01:53 -07:00
Chad Little
00796e592b Move Setup Issues into it's own notification style menu
Summary: Ref T11132. This gets rid of the red bar for admins and instead shows a new menu item next to notifications/chat if there are unresolved configuration issues. Menu goes away if there are no issues. May move this later into the bell icon, but think think might be the right place to start especially for NUX and updates. Maybe limit the number of items?

Test Plan:
Tested with some, lots, and no config issues.

{F1790156}

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T11132

Differential Revision: https://secure.phabricator.com/D16461
2016-08-29 10:43:30 -07:00
epriestley
8c41226624 Survive receipt of too much workboard column position information
Summary:
Fixes T11468. I was able to find //a// reproduction case for the issue, at least, described in my comments there.

When the server sends back position information for columns we aren't displaying, for whatever reason, just ignore that information.

Future work in T4900 to do more synchronization may revisit this.

Test Plan: See T11468#190177 for steps. Did that stuff with this patch, got a clean edit.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T11468

Differential Revision: https://secure.phabricator.com/D16397
2016-08-14 09:13:08 -07:00
Chad Little
b53d38315f Fix Differental Filetree browser hotkey
Summary: Adds a class for explicitly hiding the sidenav.

Test Plan: Set Config to Enable Filetree. View a diff, see tree. Press `f`, see it go away. Reload page, see persistence.

Reviewers: avivey, epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D16359
2016-08-01 14:36:48 -07:00
epriestley
aee9d88c17 Give the Calendar month view a nice hover effect
Summary:
Ref T11326. Currently, we link Calendar days using hidden DOM nodes.

This is nice because it's simple, and right-clicking a day works properly. However, it's a bit ugly/unintuitive, messy, and unclear. It's especially messy because days are really two different rows, one for events and one for day/week numbers.

Instead, use JS to highlight day cells. You can still right-click by clicking the actual day number, which seems like a reasonable compromise.

Test Plan: {F1738941}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T11326

Differential Revision: https://secure.phabricator.com/D16334
2016-07-27 09:36:07 -07:00
epriestley
e5256bd815 Hide time controls when editing all-day Calendar events
Summary:
Ref T11326. When an event is all-day, hide the time controls for the start/end dates. These aren't used and aren't helpful/useful.

This got a little more complicated than it used to be because EditEngine forms may have only some of these controls present.

Test Plan: Edited an all-day event; edited a normal event; swapped an event between normal and all-day.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T11326

Differential Revision: https://secure.phabricator.com/D16327
2016-07-27 05:01:19 -07:00
epriestley
4859a33739 Make Calendar day view a little more consistent
Summary: Ref T11326. This just cleans things up a little and removes some of the obvious layout/CSS issues.

Test Plan:
- Viewed day view before/after. Also viewed profile panel.

Before:

{F1725547}

After:

{F1725548}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T11326

Differential Revision: https://secure.phabricator.com/D16308
2016-07-15 14:24:29 -07:00
epriestley
8062423271 Respect 12 hour vs 24 hour time formats in Calendar day views
Summary: Fixes T9202.

Test Plan:
  - Viewed day in 12-hour, saw "8:00 PM".
  - Viewed day in 24-hour, saw "16:00".

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T9202, T10932

Differential Revision: https://secure.phabricator.com/D16290
2016-07-13 15:39:20 -07:00
Ben Schmidt
dea952aa48 Make links to hidden comments work (Fixes T11117) 2016-07-09 18:07:37 +10:00
Aviv Eyal
1a303e7d2a Make "/" focus the search input again
Summary:
See D1902, T989, T11263, D15984, T4103 , D15976, https://secure.phabricator.com/w/changelog/2016.22/, T2527, T11231, T8286, T11264 for discussion!

When we get another copy of T989, I will rename it to "Build a complicated keybinding settings page like a cool video game" and leave it open forever.

Test Plan: Pressed "/" in Firefox, had my pristine browsing experience inexplicably hijacked by this horrible application.

Reviewers: avivey, chad

Reviewed By: avivey

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D15984
2016-07-08 14:18:35 -07:00
epriestley
444e353700 Fix "\" keyboard shortcut on German keyboard layouts
Summary: Ref T10252. This is similar to D16259, but makes KeyboardShortcutManager more relaxed about `altKey` when typing obscure characters.

Test Plan: Pressed Option + Shift + 7 on a German keyboard layout, saw Conphernece sidebar toggle.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10252

Differential Revision: https://secure.phabricator.com/D16260
2016-07-08 08:39:34 -07:00
epriestley
a8a9fddb07 Trigger autocomplete when "@" is typed on German keyboards
Summary:
Ref T10252. On the German keyboard layout, you must type "Alt" + "L" to generate an "@" character.

We currently ignore this event, assuming it's a keyboard command. However, I think we can safely continue so that autocomplete works on German layouts.

Test Plan:
  - Switched keyboard layout to German.
  - Typed Alt + L to generate an "@".
  - Typed some username text.
  - Got autocompleter.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10252

Differential Revision: https://secure.phabricator.com/D16259
2016-07-08 08:36:35 -07:00
epriestley
7b5e84282f Improve "thread" rendering of unusually-shaped graphs
Summary:
Ref T4788. This fixes all the bugs I was immediately able to catch:

  - "Directory-Like" graph shapes could draw too many vertical lines.
  - "Reverse-Directory-Like" graph shapes could draw too few vertical lines.
  - Terminated, branched graph shapes drew the very last line to the wrong place.

This covers the behavior with tests, so we should be able to fix more stuff later without breaking anything.

Test Plan:
  - Added failing tests and made them pass.

{F1708158}

{F1708159}

{F1708160}

{F1708161}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T4788

Differential Revision: https://secure.phabricator.com/D16216
2016-07-01 11:15:24 -07:00
epriestley
2c43d055b1 Remove old ObjectBox tab cruft
Summary: Ref T10628. Cleans up remaining weird, unused tab behaviors in ObjectBoxView to simplify ObjectBox.

Test Plan: Toggled tabs in Files.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10628

Differential Revision: https://secure.phabricator.com/D16208
2016-06-30 18:50:25 -07:00
epriestley
7a315780b4 When using the "Close as Duplicate" relationship action, limit the UI to 1 task
Summary:
Ref T4788. When closing a task as a duplicate of another task, you can only select one task, since it doesn't really make sense to merge one task into several other tasks (this operation is //possible//, but probably not what anyone ever wants to do, I think?).

Make the UI understand this: after you select a task, disable all of the "select" buttons in the UI to make this clear.

Test Plan:
  - Used "Close as Duplicate", only allowed to select 1 task.
  - Used other editors like "Merge Duplicates In", allowed to select lots of tasks.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T4788

Differential Revision: https://secure.phabricator.com/D16203
2016-06-30 13:48:21 -07:00
epriestley
4bbe6f307a Resolve relationship edit conflicts more naturally
Summary:
Ref T11179. Ref T4768. Currently, on `master`, if two users open "Edit Revisions" at the same time, then add revisions A and B, only the last state wins (just "B").

Instead, apply these as "add A" and "add B" so they merge in a natural way.

Test Plan:
  - Opened edit dialog in two windows.
  - Added "A" in one, "B" in the other.
  - Saved both.
  - Saw "Added A" and "Added B" transactions, instead of "Added A" and "Removed A, added B".

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T4768, T11179

Differential Revision: https://secure.phabricator.com/D16164
2016-06-22 11:17:30 -07:00
epriestley
56d3197fe0 Fold task-relationship actions into an accordion dropdown
Summary:
Ref T11179. Alternative to D16152. I think this turned out a bit better than the other one did.

Currently, we render two copies of the menu (one for mobile, one for desktop). A big chunk of this is sharing the nodes instead: when you open the mobile dropdown menu, it steals the nodes from the document. When you close it, it puts them back. Magic! Sneaky!

Test Plan:
{F1695499}

{F1695500}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T11179

Differential Revision: https://secure.phabricator.com/D16157
2016-06-20 19:14:27 -07:00
epriestley
ad0562e15e Improve some typeahead matching behaviors
Summary:
Ref T8510. Sort prefix matches above non-prefix matches, so that "Ape Discovery" does not match "discovery" better than "Discovery".

Sort functions last.

Rename function internal strings so they don't get over-promoted the prefix-match rules.

Add kind of a hack to get "Project X" sorting above all the "Project X (Milestone 1)" results.

Test Plan:
Created "Ape Discovery", "Baboon Discovery", "Chimpanzee Discovery", etc.

Main project now sorts above milestones:

{F1681773}

Prefix matches now sort above other matches:

{F1681774}

Function results (rarely used) are now less prominent:

{F1681775}

Better function results here:

{F1681776}

More function results:

{F1681777}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T8510

Differential Revision: https://secure.phabricator.com/D16094
2016-06-09 13:22:34 -07:00
epriestley
4d12c58dcf Allow Pholio mocks to be created and edited without drag-and-drop
Summary: Ref T6523. Allows you to click stuff instead of using drag-and-drop.

Test Plan: On iOS simulator, created and updated a mock.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T6523

Differential Revision: https://secure.phabricator.com/D16088
2016-06-09 08:43:38 -07:00
Asher Baker
f0eb6f4fe0 Add client-side check for protocol mismatch
Summary:
Fixes T10402.
I tried about 50 variations on the wording and notification layout, this seemed by far the most reasonable.
Didn't implement a way to ignore the warning, which might be required - but figured this is serious and broken enough while being completely invisible 99% of the time that it's worth shouting about.

Test Plan: Messed around with $_SERVER['HTTPS'] on the server side and client_uri on the client side - saw reasonable results in all combinations.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: epriestley

Maniphest Tasks: T10402

Differential Revision: https://secure.phabricator.com/D16064
2016-06-07 15:54:48 +00:00
epriestley
a4e5780043 Remove "Search Preferences"
Summary:
Ref T4103. This removes these options:

{F1660585}

The jump nav option came from T916, when we had a separate jump nav on the home page. Essentially no one has ever been confused by the behavior of search or disabled this feature. Here are the stats for this install:

| Total Users | 36656 |
| Have Set Any Preference | 3084 |
| Have Disabled Jump | 6
| Are Not "Security Researchers" | 2
| Any Account Activity | 0

The "/" option came in the same change, but the preference came from T989. This keystroke conflicts with a default Firefox keystroke. Almost no one cares about this either, but I count 6 real users who have disabled the behavior. I suspect the number of real users who //use// it may be smaller.

In Safari and Firefox, the "tab" key does the same thing.

In Chrome, the "tab" key does the same thing if {nav Preferences > Web Content > "Pressing Tab highlights..."} is disabled.

Upshot: jump nav is great, bulk of the change in T989 was clearly great, specific preferences that came out of it seem not-so-great and now is a good time to kill them as we head into T4103.

Test Plan:
  - Grepped for removed constants.
  - Pressed "/".
  - Searched for `T123`.
  - Viewed settings.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T4103

Differential Revision: https://secure.phabricator.com/D15976
2016-05-26 06:21:47 -07:00
epriestley
2a00f185eb When the JS "Intl" API is available, use it to guess the timezone
Summary:
Ref T3025. Chrome gives us an easily-accessible, much better guess at which timezone the user is in.

Firefox also exposes "Intl" but this doesn't seem to be a reliable method to read the timezone.

Test Plan:
In Chrome, swapped my system date/time between zones, clicked the "reconcile" popup, got the dropdown prefilled accurately.

In Safari (no `Intl` API) got the normal flow with no default selected.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T3025

Differential Revision: https://secure.phabricator.com/D15962
2016-05-22 09:14:02 -07:00
epriestley
a91004ef1b Detect timezone discrepancies and prompt users to reconcile them
Summary: Ref T3025. This adds a check for different client/server timezone offsets and gives users an option to fix them or ignore them.

Test Plan:
  - Fiddled with timezone in Settings and System Preferences.
  - Got appropriate prompts and behavior after simulating various trips to and from exotic locales.

In particular, this slightly tricky case seems to work correctly:

  - Travel to NY.
  - Ignore discrepancy (you're only there for a couple hours for an important meeting, and returning to SF on a later flight).
  - Return to SF for a few days.
  - Travel back to NY.
  - You should be prompted again, since you left the timezone after you ignored the discrepancy.

{F1654528}

{F1654529}

{F1654530}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T3025

Differential Revision: https://secure.phabricator.com/D15961
2016-05-21 13:25:23 -07:00
Chad Little
e902fc0e2a Move Tablet breakpoint from 768 -> 920
Summary: Bumping this up higher since two column views get extra tight fast below 900 px. This felt most correct to me, dialing it back from first attempt at 960. Mostly I don't want to ever accidentally trigger it when I'm on the 12" MacBook. Ref T10926

Test Plan: Durable Column, Workboards, Dashboards, Tasks.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: avivey, Korvin

Maniphest Tasks: T10926

Differential Revision: https://secure.phabricator.com/D15960
2016-05-21 12:11:27 -07:00