1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-12-20 12:30:56 +01:00
Commit graph

719 commits

Author SHA1 Message Date
Chad Little
21f0ce7362 Make taller tables the default for AphrontTableView
Summary: I considered at the time just making all tables taller. This removes the special casing and adds the space universally. On first glance all smaller tables look great, but Diffusion seems a little bloated. After a short time period though that went away for me. I do think Diffusion overall needs a UI refresh.

Test Plan: Tested numerous tables in Phortune, Diffusion, etc. Spacing feels more readable.

Reviewers: btrahan, epriestley

Reviewed By: epriestley

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D12328
2015-04-08 08:38:15 -07:00
epriestley
2ecdbbec10 Don't revert Conpherence page titles when updating notifications
Summary: Fixes T7709. `JX.Title` deals with adding `(1)`, etc., counts, so send updates through it.

Test Plan: Clicked between some threads in Conpherence, no title flickering.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7709

Differential Revision: https://secure.phabricator.com/D12260
2015-04-02 13:41:48 -07:00
Chad Little
8e5153e866 Make busy indicator durable column aware
Summary: Fixes T7699, moves busy indicator when durable column is present

Test Plan: visit uiexamples busy page, toggle column on and off

Reviewers: btrahan, epriestley

Reviewed By: epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T7699

Differential Revision: https://secure.phabricator.com/D12257
2015-04-01 20:10:32 -07:00
epriestley
ba10e8ed41 Reduce cleverness of tablet breakpoint adjustment
Summary:
Fixes T7713. 2 much math X.X

Specifically, we could end up with some nonsense widths here (smaller than the phone breakpoint) with an initially-hidden column.

Test Plan: Hit tablet breakpoint with column open and closed.

Reviewers: chad, btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7713

Differential Revision: https://secure.phabricator.com/D12225
2015-03-31 13:44:49 -07:00
Bob Trahan
f75248de6f Conpherence - refine menu interactions with respect to "Show More"
Summary:
Ref T7566. Prior to this diff, we had a broken mess in the "Messages" section. Now, "Messages" behave like rooms in that whatever is loaded at page load time is at the top of the list.

Additionally, refine "show more" behavior such that it simply shows the next X, but if there exists X + 1 then we have another "show more" that kicks you to application search. Theoretically, there are still corner cases where users are in a ton of rooms or a ton of messages respectively, but this feels pretty good.

Consolidates title rendering code so we always render the list of participants and no more "No Title".

Also remove the policy icons for messages consistently, helping to differentiate them from rooms at a glance.

Test Plan: clicked around in conpherence main - looked good. tried "show more" and it worked! played around in durable column and things seemed reasonable there too.

Reviewers: chad, epriestley

Reviewed By: epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T7566

Differential Revision: https://secure.phabricator.com/D12222
2015-03-31 12:45:32 -07:00
Chad Little
a0bf9a290a Use font-awesome for busy indicator
Summary: Testing out using a gear instead of compass gif and background. Let me know how it feels

Test Plan:
UIExamples busy page

{F353571}

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D12201
2015-03-29 16:45:18 -07:00
epriestley
69020af0c7 Make column show/hide behaviors a little simpler
Summary: Ref T7062. The previous fix caused an extra, unnecessary thread load on mobile. Make this code a bit simpler and fix the unnecessary load.

Test Plan: No more load on mobile.

Reviewers: btrahan, chad

Reviewed By: chad

Subscribers: epriestley

Maniphest Tasks: T7062

Differential Revision: https://secure.phabricator.com/D12196
2015-03-29 07:27:06 -07:00
epriestley
a48b486344 Fix a race condition with the chat column
Summary:
Ref T7062. When we load a thread, we always show the column, even if it has been closed between the time we sent the request and when we're processing the response.

Normally this isn't a big deal, but it can specifically show up on mobile.

(This load also shouldn't be happening at all, but I'll fix that separately.)

Test Plan: Mobile no longer shows the column after it loads.

Reviewers: btrahan, chad

Reviewed By: chad

Subscribers: epriestley

Maniphest Tasks: T7062

Differential Revision: https://secure.phabricator.com/D12195
2015-03-28 15:46:13 -07:00
epriestley
95efd50470 Remove stray debugging code
Auditors: chad
2015-03-28 07:14:14 -07:00
epriestley
8c1a1312e8 Improve column device behaviors
Summary:
Fixes T7062. When the column is open, we only want to consider the screen width which is avilable for content when computing responsive breakpoints.

Specificially, if you have a 1000px wide browser window (normally "desktop") but the column is open (300px) so you only have 700px free for content (normally "tablet"), we should drop to the tablet breakpoint. This lets you have a narrow column of "tablet" content next to the chat column, instead of a really squished column of "desktop" contnet.

This also means the chat column can't directly use JX.Device to hide itself.

Test Plan: Resized screen with column open, saw content go from Desktop + Column -> Tablet + Column -> Tablet -> Mobile.

Reviewers: btrahan, chad

Reviewed By: chad

Subscribers: epriestley

Maniphest Tasks: T7062

Differential Revision: https://secure.phabricator.com/D12189
2015-03-28 06:54:23 -07:00
epriestley
a17542ab28 Touch up PHP/JS interactions for inline comments
Summary:
Ref T1460. Overall:

  - Pass `objectOwnerPHID` consistently.
  - Pass viewer consistently.
  - Set the correct draft state for checkboxes on the client.

Test Plan:
  - Made inline comments in Differential.
  - Made inline comments in Diffusion.

Reviewers: chad

Reviewed By: chad

Subscribers: epriestley

Maniphest Tasks: T1460

Differential Revision: https://secure.phabricator.com/D12186
2015-03-27 17:08:31 -07:00
Bob Trahan
e4b7263bf8 Conpherence - Differentiate audience of Threads/Rooms with icon
Summary:
Fixes T7629 plus an un filed bug that's breaking creating new threads since we need to add participants EVEN EARLIER than we were doing it now that policy is actually enforced.

Back to the main thrust of this, there is one UI corner case - in the main view if you go from 1:1 to 1:1:1 (i.e. add a 3rd recipient, or Nth in a row) the icon only updates on page reload. I figure this will get sorted out at a later refactor as we make the client better / share more code with durable column.

One other small behavioral oddity is in the main view sometime we start loading with no conpherence. in that case, rather than show some incorrect icon, we show no icon (and "no title") and then things change at load. Seems okay-ish.

Finally, @chad - the CSS is a very work-man-like "use the built in stuff you can specify from PHP" so I'm sure it needs some love.

Test Plan: made all sorts of rooms and threads and liked the icons. noted smooth loading action as i switched around

Reviewers: chad, epriestley

Reviewed By: epriestley

Subscribers: Korvin, chad, epriestley

Maniphest Tasks: T7629

Differential Revision: https://secure.phabricator.com/D12163
2015-03-26 12:24:29 -07:00
Bob Trahan
42a0229a52 Conpherence - Implement edit rules for rooms
Summary: Fixes T7586. If you can't edit a room, the pertinent UI is greyed out. One exception is the title of the room in the full viewer; this crumb is not disabled as it would be hard to read. Otherwise though, everything is disabled nicely.

Test Plan: tried to add participants when I wasn't allowed to and got an error. added participants otherwise okay. tried to edit title when i wasn't allowed and got an error. otherwise okay. left conpherence threads / rooms successfully.

Reviewers: epriestley, chad

Reviewed By: epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T7586

Differential Revision: https://secure.phabricator.com/D12161
2015-03-25 11:48:22 -07:00
epriestley
1fd163d097 Mostly provide CSS for "done" states
Summary: Ref T7660. I'm not toggling "inline-state-is-draft" correctly in JS yet since it's a little tricky (you can reload to see it) but the main state should work.

Test Plan:
  - Clicked "done", saw comment opacity fade with placeholder style.

Reviewers: chad

Reviewed By: chad

Subscribers: epriestley

Maniphest Tasks: T7660

Differential Revision: https://secure.phabricator.com/D12160
2015-03-25 10:57:08 -07:00
epriestley
c03297ab5a Fix interaction between undo and inline comment placement
Summary:
Fixes T7658. Currently, we remove the "undo" before placing the comment, but that causes us to lose track of which row we should be examining.

Instead, place the comment first, then remove the "undo".

Test Plan: This stuff is hard to test comprehensively, but the original report reproduced easily and is now fixed. I wasn't able to break anything by adding/editing/deleting comments.

Reviewers: btrahan, chad

Reviewed By: chad

Subscribers: epriestley

Maniphest Tasks: T7658

Differential Revision: https://secure.phabricator.com/D12157
2015-03-25 07:14:12 -07:00
Bob Trahan
25767096c9 Conpherence - implement join / view rules for rooms
Summary:
Ref T7585. This implements everything specified, with a few caveats

- since rooms you have yet to join can't be viewed in the column yet, the column view has some bugs and isn't expected to work.
- the room you're looking at is just pre-pending to the top of the "recent" list

Test Plan: made a room that no one could join. verified when viewing that there was no comment ui. made a room that others could join. verified folks who had yet to join had a "join" button with an area for text. tried joining with / without message text and it worked in both cases

Reviewers: chad, epriestley

Reviewed By: epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T7585

Differential Revision: https://secure.phabricator.com/D12149
2015-03-24 18:38:16 -07:00
epriestley
21826ed7b3 Don't highlight very large files by default
Summary:
Ref T5644. See some discussion in D8040.

When a file is very large (more than 64KB of text), don't activate syntax highlighting by default. This should prevent us from wasting resources running `pygmentize` on enormous files.

Users who want the file highlighted can still select "Highlight As...".

The tricky part of this diff is separating the headers into "changeset" headers and "undershield" (rendering) headers. Specifically, a file might have these headers/shields:

  - "This file is newly added."
  - "This file is generated. Show Changes"
  - "Highlighting is disabled for this large file."

In this case, I want the user to see "added" and "generated" when they load the page, and only see "highlighting disabled" after they click "Show Changes". So there are several categories:

  - "Changeset" headers, which discuss the changeset as a whole (binary file, image file, moved, added, deleted, etc.)
  - "Property" headers, which describe metadata changes (not relevant here).
  - "Shields", which hide files from view by default.
  - "Undershield" headers, which provide rendering information that is only relevant if there is no shield on the file.

Test Plan:
  - Viewed a diff with the library map, clicked "show changes", got a "highlighting disabled" header back with highlighting disabled.
  - Enabled highlighting explicitly (this currently restores the shield, which it probably shouldn't, but that feels out of scope for this change). The deshielded file is highlighted per the user's request.
  - Loaded context on normal files.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: joshuaspence, epriestley

Maniphest Tasks: T5644

Differential Revision: https://secure.phabricator.com/D12132
2015-03-24 05:26:39 -07:00
epriestley
4310c4ed53 Track a "Done" state on inline comments
Summary:
Ref T1460. This just barely works, but throwing it up in case any of it sounds mechanically crazy before we build integrations/UI/etc.

Specifically, these are the behaviors:

  - You can mark your own draft comments as "done" before you submit them. The intent is to let reviewers mark their stuff advisory/minor/not-important before they submit it, to hint to authors that they don't expect the feedback to necessarily be addressed (maybe it's a joke, maybe it's just discussion, maybe it's "consider..").
  - You can mark others' published comments as "done" if you're the revision/commit author. The intent is to keep this lightweight by not requiring an audit trail of who marked what done when. If anyone could mark anything done, we'd have to have some way to show who marked stuff.
  - When you mark stuff done (or unmark it), it goes into a "draft" state, where you see the change but others don't see it yet. The intent is twofold:
    - Be consistent with how inlines work.
    - Allow us to publish a "epriestley updated this revision + epriestley marked 15 inlines as done" story later if we want. This seems more useful than publishing 15 "epriestley marked one thing as done" stories.
  - The actual bit where done-ness publishes isn't implemented.
  - UI is bare bones.
  - No integration with the rest of the UI yet.

Test Plan: Clicked some checkboxes.

Reviewers: chad, btrahan

Reviewed By: btrahan

Subscribers: paulshen, chasemp, epriestley

Maniphest Tasks: T1460

Differential Revision: https://secure.phabricator.com/D12033
2015-03-24 05:26:11 -07:00
Bob Trahan
441112c8e2 Conpherence - fix a typo
Summary: this typo broke (at least) renaming the thread from the durable column.

Test Plan: renamed a thread from durable column and it worked

Reviewers: chad, epriestley

Reviewed By: epriestley

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D12104
2015-03-17 17:01:33 -07:00
Bob Trahan
8fe78c9029 Conpherence - update the "unread" count in notifications panel aggressively
Summary:
Fixes T7561. Basically everytime we load some transactions in the thread manager, kick off an async thread to update the notification panel.

Should I consolidate this little bit of code into something like this._handleTransactionResponse(r)... ?  I just want to keep the JS clear for other engineers and I wasn't sure if that was hiding a bit too much detail.

Test Plan: user a opened durable column. user b sent user a a few messages. reloaded user a page and noted the "N" count became N-1 as the message loaded. Switched messages and saw N-2, N-3, etc as I loaded up the messages.

Reviewers: chad, epriestley

Reviewed By: epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T7561

Differential Revision: https://secure.phabricator.com/D12099
2015-03-17 13:40:36 -07:00
Bob Trahan
98899c822e Conpherence - kill race conditions around update
Summary:
Fixes T6713. The idea is to keep checking what's going on in the update paths that touch the DOM. If we're doing an update or should be doing a different update, then we bail early.

This is the type of code + testing that makes me dizzy after awhile, but I think it works...

Test Plan:
added a "forceStall" parameter to the column view controller, which when specified sleeps for seconds before returning. I then augmented the JS such that the "send message" code for the durable column would specifiy this parameter.

For actual testing, I then spammed the heck out of the durable column channel and saw each message only once. I also spammed the column, switched browsers to a user on the same thread in the normal "speedy" view, sent messages there, and also only received one copy

Reviewers: chad, epriestley

Reviewed By: epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T6713

Differential Revision: https://secure.phabricator.com/D12092
2015-03-16 16:35:05 -07:00
Bob Trahan
c21301d153 Conpherence - fix 0 messages in header race condition
Summary:
Fixes T7545. Turns out we had the right logic to handle this basically, and just needed to variablize the CSS class that gets added / removed as appropos.

Note the new behavior is to keep the icon highlighted just with no number. This emulates how it would work if e.g. there was no unread message in the first place and you just clicked the message icon to invoke the message menu.

Test Plan: had a durable conpherence open for user A with user B. used a separate browser to send message as user B. reloaded as user A - saw new message in conpherence durable column and the "1" unread icon. I then clicked the "1" and saw it disappear as expected

Reviewers: chad, epriestley

Reviewed By: epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T7545

Differential Revision: https://secure.phabricator.com/D12091
2015-03-16 14:28:25 -07:00
epriestley
cf54194ef5 Improve chat column behavior for newline in empty text box
Summary: Ref T7538. I got this half correct but not fully correct: when you press enter in an empty text box, do nothing (instead of: sending an empty message, or writing a literal newline).

Test Plan: Hit enter in empty chat column box.

Reviewers: btrahan, chad

Reviewed By: chad

Subscribers: epriestley

Maniphest Tasks: T7538

Differential Revision: https://secure.phabricator.com/D12089
2015-03-16 06:44:07 -07:00
epriestley
1773af6ada Enable the chunk storage engine
Summary: Ref T7149. This works now, so enable it.

Test Plan:
  - Uploaded large and small files in Firefox, Safari and Chrome.
  - Uploaded large files with `arc upload`.
  - Stopped/resumed large files with all clients.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7149

Differential Revision: https://secure.phabricator.com/D12079
2015-03-15 11:37:05 -07:00
epriestley
e5580d038d Improve Conpherence column textarea JS behaviors
Summary:
  - Don't show a loading state on the whole column while sending chat. We could show some kind of minor loading state, but standard JX.Busy stuff will kick in after a couple seconds anyway.
  - Blank the textarea immediately on submit so you can start typing more text.
  - Don't disable the form while submiting; disabling it prevents you from typing more text.
  - Hide the placeholder while the textarea is focused. If we don't do this, the placeholder reappearing after submitting text feels weird to me.

Test Plan:
  - Sent a lot of text.
  - Real fast.
  - Focused and unfocused the area.

Reviewers: btrahan, chad

Reviewed By: chad

Subscribers: epriestley

Differential Revision: https://secure.phabricator.com/D12086
2015-03-14 12:00:17 -07:00
epriestley
e6f1c34a47 Send on enter in Conpherence chat column
Summary: Ref T7538. We can figure out whether to backport this to main Conpherence later and/or remove buttons, etc., but this behavior seems pretty clearly good.

Test Plan:
  - Pressed enter (sent message).
  - Pressed shift+enter (newline).

Reviewers: btrahan, chad

Reviewed By: chad

Subscribers: epriestley

Maniphest Tasks: T7538

Differential Revision: https://secure.phabricator.com/D12085
2015-03-14 11:16:47 -07:00
epriestley
7d69d8ae6a Remove support for Balanced payments
Summary: See <https://www.balancedpayments.com/stripe>. Just get rid of support since Phortune is a prototype anyway.

Test Plan: `grep`, poked around Phortune.

Reviewers: btrahan, chad

Reviewed By: chad

Subscribers: aurelijus, epriestley

Differential Revision: https://secure.phabricator.com/D12074
2015-03-13 15:47:37 -07:00
epriestley
135280be9e Support HTML5 / Javascript chunked file uploads
Summary:
Ref T7149. This adds chunking support to drag-and-drop uploads. It never activates right now unless you hack things up, since the chunk engine is still hard-coded as disabled.

The overall approach is the same as `arc upload` in D12061, with some slight changes to the API return values to avoid a few extra HTTP calls.

Test Plan:
  - Enabled chunk engine.
  - Uploaded some READMEs in a bunch of tiny 32 byte chunks.
  - Worked out of the box in Safari, Chrome, Firefox.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7149

Differential Revision: https://secure.phabricator.com/D12066
2015-03-13 11:30:36 -07:00
Bob Trahan
e137d0a0c5 Conpherence - finesse global upload vs conpherence durable column upload
Summary: Fixes T7539. We need to set the "with-column" css class on the document body to make things like the jx-mask style-able. Also, make the global upload control only do it for the standard phabrcator page and not the document body.

Test Plan: dragged a file to conpherence column and it worked! uploaded a file to homepage with column open and it worked! uploaded a file to /file/ with column open and it worked!

Reviewers: chad, epriestley

Reviewed By: epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T7539

Differential Revision: https://secure.phabricator.com/D12055
2015-03-12 13:40:06 -07:00
Bob Trahan
eed10e47c9 Conpherence - support device in durable column
Summary:
Ref T7014. This makes it so

 - you can't invoke the column from a device
 - if you are in desktop size and resize to tablet or phone, the column closes.
 - if you resize desktop -> device -> desktop, the column closes at device size and reopens at desktop size
 - if you load, then resize device -> desktop the column opens if the user has that preference
 - there is a brief flicker when you load on 'device' with the column open preference. it lasts as long as the js stack takes to calculate the device css rule.

Test Plan: see summary but i did stuff to do all that

Reviewers: chad, epriestley

Reviewed By: epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T7014

Differential Revision: https://secure.phabricator.com/D12052
2015-03-12 12:23:31 -07:00
Bob Trahan
d9adedd9d6 Conpherence - smooth out user interaction switching threads in the durable column
Summary: Ref T7014. This changes the title and selected icon right as the user clicks it. This could //maybe// be in the "willLoadThread" callback hook, but it doesn't happen every time we load a thread, just **this** time so keep it right in the listener for now.

Test Plan: switched some threads and liked what I saw

Reviewers: epriestley, chad

Subscribers: Korvin, epriestley

Maniphest Tasks: T7014

Differential Revision: https://secure.phabricator.com/D12043
2015-03-11 15:55:35 -07:00
Bob Trahan
194a183ea6 Conpherence - add draft support to column and refine sending message user workflow
Summary: Ref T7014. This diff adds draft support.

Test Plan: made many comments and they submitted nicely.

Reviewers: chad, epriestley

Reviewed By: epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T7014

Differential Revision: https://secure.phabricator.com/D12046
2015-03-11 15:54:07 -07:00
Bob Trahan
c85c07012e Conpherence - make Hide Column action save user preference
Summary: Fixes T7532.

Test Plan: clicked it, reloaded it, still closed

Reviewers: epriestley, chad

Reviewed By: chad

Subscribers: Korvin, epriestley

Maniphest Tasks: T7532

Differential Revision: https://secure.phabricator.com/D12044
2015-03-11 12:58:51 -07:00
epriestley
a1820ecf6e Remove Konami code easter egg
Summary:
This is cool in theory, but has broken like 5 times and is broken now too. The CSS magic just isn't robust enough to keep up with CSS changes.

Just strip it out for now; if we come up with some more durable replacement we can put that back in its place.

Test Plan: Typed konami code, page didn't break horribly.

Reviewers: chad, btrahan

Reviewed By: btrahan

Subscribers: epriestley

Differential Revision: https://secure.phabricator.com/D12039
2015-03-10 18:46:28 -07:00
epriestley
88b46063b4 Make durable column sticky across requests
Summary: When you open the column, keep it open on future requests.

Test Plan: Opened column, clicked to Conpherence (no column), clicked elsewhere (column again), reloaded page (column), closed column, clicked something (no column).

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Differential Revision: https://secure.phabricator.com/D12038
2015-03-10 18:46:16 -07:00
epriestley
6fa507987d Generalize URI pattern blacklist for Quicksand
Summary:
Fixes T7060. Removes some hard-coding.

This assumes that "pages with no durable column" and "pages with no Quicksand" are the same, but that's correct today and I can't come up with a use case where they'd be different offhand.

Test Plan:
  - Clicked a revision with column open, got Quicksand navigation.
  - Clicked into Conpherence with column open, got real navigation.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7060

Differential Revision: https://secure.phabricator.com/D12036
2015-03-10 15:32:15 -07:00
Bob Trahan
4674a5eab9 Conpherence - add icon-based room switcher to durable column view
Summary: Ref T7380. This does the most basic thing ever and sticks up to 6 icons in there.

Test Plan: clicked the icons and noted new conpherences loaded in nicely

Reviewers: epriestley, chad

Reviewed By: epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T7380

Differential Revision: https://secure.phabricator.com/D12037
2015-03-10 15:30:49 -07:00
epriestley
5afa2c3b62 Add support for playing sounds
Summary:
Ref T5369. New HTML5 version without flash dependencies.

This doesn't play any sounds.

Test Plan: Did not play any sounds.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: joshuaspence, epriestley

Maniphest Tasks: T5369

Differential Revision: https://secure.phabricator.com/D9535
2015-03-10 14:20:00 -07:00
Bob Trahan
10f1692b1b Conpherence - more threadManager stuff and get scrolling working
Summary: Ref T7014. The main conpherence view is kind of broken without this in subtle ways because of /conpherence/ versus /conpherence/x/ init'ing things differently; this fixes that. Moves more normal view conpherence logic into threadManager. Makes all the display code happen outside of threadManager, setting us up for some display manager later maybe.

Test Plan: sent messages, updated title, etc and the messages pane auto scrolled correctly!

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T7014

Differential Revision: https://secure.phabricator.com/D12035
2015-03-10 13:53:30 -07:00
Bob Trahan
2373185e9b Conpherence - introduce ConpherenceThreadManager
Summary:
Ref T7014. Fixes T7473. This adds a class to handle thread state about what thread is loaded and what transaction we've seen last. It is deployed 100% in the durable column and only partially deployed in the regular view. Future diff(s) should clean up regular view. Note ConpherenceThreadManager API might change a bit at that time.

Also includes a bonus bug fix so logged out users can't toggle this column

Test Plan: tried to use durable column while logged out and nothing happened. sent messages, aphlict-received messages, added people, and changed title from both views

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T7473, T7014

Differential Revision: https://secure.phabricator.com/D12029
2015-03-10 12:20:29 -07:00
epriestley
dd501117e8 When deleting inline comments, offer "undo" instead of prompting
Summary:
Ref T2009. Ref T1460.

Fixes T2618. When users hit "Delete" on inline comments, delete immediately and offer them "Undo". If they delete indirectly (e.g., by clicking "Delete" from the preview at the bottom of the page), we still prompt them, because the "Undo" action either won't be available or may not be easy to find. This is a "refdelete".

Fixes T6464. This was just a mess. Make it not as much of a mess. It should work now. Pretty sure.

Fixes T4999. We did not refresh these links often enough to find targets for them, so they could race with content. Reevaluate them after loading new changes.

Test Plan:
  - Deleted and undid deletion of inlines from main view and preview.
  - Clicked "View" on inlines.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T6464, T4999, T2618, T1460, T2009

Differential Revision: https://secure.phabricator.com/D12032
2015-03-09 17:27:51 -07:00
Bob Trahan
9b9a8001fa Conpherence - add ability to change title in the durable column
Summary: Ref T7014.

Test Plan: changed the conpherence title from the column. since i can't get scrolling to work, i inspect the dom to verify the title change transaction showed up properly

Reviewers: epriestley, chad

Reviewed By: epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T7014

Differential Revision: https://secure.phabricator.com/D12002
2015-03-09 12:09:43 -07:00
epriestley
56a9709008 Reduce code duplication for inline "Undo"
Summary:
Ref T2009. This is another almost-identical copy of the row scaffolding, which has the same 1up/2up bugs as the 8 other copies of this code.

Turn the "undo" element into an InlineCommentView so we can scaffold it.

Then, scaffold it with the same code as everything else.

Test Plan: Hit "Undo", swapped from 1up to 2up, hit "undo" again, swapped back, tried left/right, everything rendered with proper scaffolding.

Reviewers: btrahan, chad

Reviewed By: chad

Subscribers: epriestley

Maniphest Tasks: T2009

Differential Revision: https://secure.phabricator.com/D12019
2015-03-09 10:26:53 -07:00
epriestley
082b7f95e6 Explicitly track inline comment reply threading
Summary:
Ref T1460. Track and store which comments are threaded replies to other comments, vs merely appearing on the same lines.

This doesn't actually write `hasReplies` yet, since that needs to happen when we un-draft comments on submission.

Test Plan: Made inline comments in Differential and Diffusion, including replies. Replies were marked as "Is Reply".

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T1460

Differential Revision: https://secure.phabricator.com/D12017
2015-03-09 10:26:50 -07:00
epriestley
c35cafda2c Use ChangesetListView on Differential standalone view
Summary:
Fixes T4452. Ref T2009. There's a hierarchy of changeset rendering power: only low-level calls, use of ChangesetDetailView, then use of ChangesetListView (a list of DetailViews).

Prior to work here, the various changeset rendering controllers got their hands dirty to varying degrees, with some using only the lowest-level rendering pipeline:

  - Phriction: no view (lowest level)
  - Diffusion: DetailView
  - Differential Changeset: DetailView
  - Differential Diff: ListView
  - Differential Revision: ListView

I brought Phriction up to use DetailView, but want to bring everything all the way up to use ListView. Each composition layer adds more features to diff browsing. In particular, this change enables "Highlight As", switching 1up vs 2up, adding inlines, etc., on the standalone view.

Test Plan:
  - Viewed a changeset standalone. Could change highlighting, switch 1up vs 2up, add and edit inlines, etc.
  - Viewed a revision; no behavioral changes.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T4452, T2009

Differential Revision: https://secure.phabricator.com/D12012
2015-03-09 10:26:47 -07:00
epriestley
1df321bf00 Fix left/right detection of inline comments in unified view
Summary:
Ref T2009. Currently, the code figures out if a comment is on the left or right by looking at the `<th />` preceeding the enclosing `<td />`.

This gets the right result in 2-up, but in 1-up rows are always `<th />`, `<th />`, `<td />`, so it always detects every inline as being in the new file.

Because "old" and "new" cells aren't inherently distingushable in the 1up view, we can't use a DOM test for this at all. Instead, just track this state explicitly.

Test Plan:
  - Made left/right comments in 1up view and 2up view.
  - Viewed them in 1up and 2up views.
  - Hovered in 1up and 2up views.
  - Diff-of-diff'd and reviewed old/new comments, then made some more.

Reviewers: btrahan, chad

Reviewed By: chad

Subscribers: epriestley

Maniphest Tasks: T2009

Differential Revision: https://secure.phabricator.com/D12011
2015-03-07 14:37:57 -08:00
epriestley
7705f452d2 Make inline comments more discoverable
Summary:
Ref T2009. Right now, when you mouse over a line number, we change the cursor to a "pointer", but that's the only hint we provide about the existence of inline comments.

Occasionally, users have reported confusion around how to leave inline comments.

Try to increase discoverability by showing the line reticle when you hover over the line.

(I could take this or leave it, but it seems OK / not annoying after 15 seconds of playing with it.)

Test Plan: Waved cursor over line numbers, attempted to test all the editor/noncontiguous/across-files cases.

Reviewers: btrahan, chad

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T2009

Differential Revision: https://secure.phabricator.com/D12010
2015-03-07 12:19:45 -08:00
epriestley
616976a45c Make inline reticle code more general and robust
Summary:
Ref T2009. Currently, the code which draws the reticle is sort of implicitly hard-coded with some of the rules for the 2up view.

Instead, use general rules:

  - Start selection at the next `<td />`.
  - End selection at the rightmost adjacent `<td />`.

These rules work in all cases.

Test Plan:
  - Activated reticle in 1up and 2up views by clicking line numbers and hovering over comments. It now draws correctly.
  - Dragged over line ranges in 1up and 2up views, saw accurate reticle.

Reviewers: btrahan, chad

Reviewed By: chad

Subscribers: epriestley

Maniphest Tasks: T2009

Differential Revision: https://secure.phabricator.com/D12009
2015-03-07 11:47:55 -08:00
Bob Trahan
c97040de47 Conpherence - get the durable column header actions working correctly
Summary:
Ref T7014. This diff addresses

 - getting it to be the right set of options
   - add participant
   - view in conpherence
   - close window
 - making those options work
 - make it so if you are on /conpherence/ you can't toggle the durable column

Test Plan: inspected dom via chrome tools and found last transaction. added a participant and inspected the single new transactin added for accuracy. used view in conpherence action to view in conpherence. used close window action to close window

Reviewers: chad, epriestley

Reviewed By: epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T7014

Differential Revision: https://secure.phabricator.com/D11991
2015-03-05 15:32:42 -08:00
epriestley
9564b0a40e Improve behavior of inline rendering with unified views
Summary:
Ref T2009. This reduces how buggy inlines are. They're still buggy.

Specifically, the inline endpoint didn't know how to scaffold inlines before, so some of them ended up rendering in the wrong rows or breaking layouts.

This passes the current renderer through to the inline editor endpoint, so it can at least get the layout correct.

Test Plan: Interacted with inlines in unified and side-by-side views.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T2009

Differential Revision: https://secure.phabricator.com/D11988
2015-03-05 14:11:51 -08:00