Summary:
Fixes T7825. If JX.Scrollbar activates, we sometimes target the wrong node.
(We don't have this issue in the column because it rebuilds a new JX.Scrollbar every time.)
Test Plan:
- Sent messages, no spooky text.
- Loaded page, got scroll to bottom.
- Unplugged all USB devices, restarted browser, repeated.
Reviewers: btrahan, chad
Reviewed By: chad
Subscribers: epriestley
Maniphest Tasks: T7825
Differential Revision: https://secure.phabricator.com/D12413
Summary:
Fixes T7558. This might not be 100% perfect but should solve most of the issue.
I briefly looked at things like `MutationObserver` (some fancy next-gen browser junk) but couldn't immediately get it working.
Other methods for handling this kind of thing involve polling, complicated polyfills, etc. We could give `MutationObserver` a more serious effort if this is too leaky.
Test Plan:
- In a thread with some images, reloaded the page and saw the scrollbar stay at the bottom.
- Tested with and without USB devices attached.
Reviewers: btrahan, chad
Reviewed By: chad
Subscribers: epriestley
Maniphest Tasks: T7558
Differential Revision: https://secure.phabricator.com/D12191
Summary: Fixes T7058. We weren't propagating `state` properly so some other code ended up doing the wrong thing.
Test Plan:
- Clicked from Home -> Anything -> Home under Quicksand, saw reloads with no double requests.
- Used "back", saw back button work properly.
Reviewers: btrahan, chad
Reviewed By: chad
Subscribers: epriestley
Maniphest Tasks: T7058
Differential Revision: https://secure.phabricator.com/D12190
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
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
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
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
Summary:
The "mlb" on the left nav creates a phantom bottom margin which gives the content measurable height but not scrollable height. Replace it with "plb" (padding) instead.
The 2px-spacer calculation was also not quite correct.
Test Plan:
- Viewed pages with navs; padding vs margin didn't seem to make any other differences.
- Scrollbar now stops in the right place in Safari, Chrome, Firefox.
Reviewers: btrahan, chad
Reviewed By: chad
Subscribers: epriestley
Differential Revision: https://secure.phabricator.com/D12007
Summary:
Fixes T7130. Fixes T7041. Fixes T7012.
Major change here is partitioning clients. In the Phacility cluster, being able to get a huge pile of instances on a single server -- without needing to run a process per instance -- is desirable.
To accomplish this, just bucket clients by the path they connect with. This will let us set client URIs to `/instancename/` and then route connections to a small set of servers. This degrades cleanly in the common case and has no effect on installs which don't do instancing.
Also fix two unrelated issues:
- Fix the timeouts, which were incorrectly initializing in `open()` (which is called during reconnect, causing them to reset every time). Instead, initialize in the constructor. Cap timeout at 5 minutes.
- Probably fix subscriptions, which were using a property with an object definition. Since this is by-ref, all concrete instances of the object share the same property, so all users would be subscribed to everything. Probably.
Test Plan:
- Hit notification status page, saw version bump and instance/path name.
- Saw instance/path name in client and server logs.
- Stopped server, saw reconnects after 2, 4, 16, ... seconds.
- Sent test notification; received test notification.
- Didn't explicitly test the subscription thing but it should be obvious by looking at `/notification/status/` shortly after a push.
Reviewers: joshuaspence, btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T7041, T7012, T7130
Differential Revision: https://secure.phabricator.com/D11769
Summary: Improves Source Sans Pro italics rendering
Test Plan: Tested a Phriction document with normal and bold italics.
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11572
Summary: Fixes T7075. The invisible "fancy" scrollbar was covering these; hide it more aggressively.
Test Plan:
- Scrollbars on Workboards can now be interacted with directly.
- Normal scrollable and unscrollable pages work as expected.
- Resized some windows.
Reviewers: btrahan, chad
Reviewed By: chad
Subscribers: epriestley
Maniphest Tasks: T7075
Differential Revision: https://secure.phabricator.com/D11560
Summary:
Fixes T7081. History here:
- JX.Scrollbar made the page scroll weird when a dialog came up because it was half-frame and half-document.
- I made it fully frame-level.
- But this wasn't really right; a better fix is to make it fully document-level.
Test Plan:
- Weird scroll on opening dialog is still fixed.
- iOS Safari no longer puts the mask over the dialog.
Reviewers: btrahan, chad
Reviewed By: chad
Subscribers: epriestley
Maniphest Tasks: T7081
Differential Revision: https://secure.phabricator.com/D11559
Summary: Builds a complete font package for browsers that support woff2. Ref T7066
Test Plan: Visit a test page locally with addtional languages.
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T7066
Differential Revision: https://secure.phabricator.com/D11545
Summary: Use `x.y` in favor of `x['y']` in //some// JavaScript callsites. Note that there are a bunch of places where the latter is explicitly used to trick `PhabricatorJavelinLinter`.
Test Plan: `arc lint`
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11442
Summary:
Fixes T7069. When jumping to a comment anchor, we get the scroll positions wrong.
Partly this is fixing some calcaulations; partly, the "show older comments" and "scroll anchor" stuff were fighting over the scroll position. Since the anchor can take care of things on its own, just let it handle stuff.
Test Plan:
- Clicked comment anchors.
- Loaded pages with anchors in the URI.
- Loaded pages with anchors hidden behind "show older comments".
In all cases, got the right scroll position.
Reviewers: btrahan, chad
Reviewed By: chad
Subscribers: epriestley
Maniphest Tasks: T7069
Differential Revision: https://secure.phabricator.com/D11540
Summary:
Ref T2086. Ref T7014. With the persistent column, there is significant value in retaining chrome state through navigation events, because the user may have a lot of state in the chat window (scroll position, text selection, room juggling, partially entered text, etc). We can do this by capturing navigation events and faking them with Javascript.
(This can also improve performance, albeit slightly, and I believe there are better approaches to tackle performance any problems which exist with the chrome in many cases).
At Facebook, this system was "Photostream" in photos and then "Quickling" in general, and the technical cost of the system was //staggering//. I am loathe to pursue it again. However:
- Browsers are less junky now, and we target a smaller set of browsers. A large part of the technical cost of Quickling was the high complexity of emulating nagivation events in IE, where we needed to navigate a hidden iframe to make history entries. All desktop browsers which we might want to use this system on support the History API (although this prototype does not yet implement it).
- Javelin and Phabricator's architecture are much cleaner than Facebook's was. A large part of the technical cost of Quickling was inconsistency, inlined `onclick` handlers, and general lack of coordination and abstraction. We will have //some// of this, but "correctly written" behaviors are mostly immune to it by design, and many of Javelin's architectural decisions were influenced by desire to avoid issues we encountered building this stuff for Facebook.
- Some of the primitives which Quickling required (like loading resources over Ajax) have existed in a stable state in our codebase for a year or more, and adoption of these primitives was trivial and uneventful (vs a huge production at Facebook).
- My hubris is bolstered by recent success with WebSockets and JX.Scrollbar, both of which I would have assessed as infeasibly complex to develop in this project a few years ago.
To these points, the developer cost to prototype Photostream was several weeks; the developer cost to prototype this was a bit less than an hour. It is plausible to me that implementing and maintaining this system really will be hundreds of times less complex than it was at Facebook.
Test Plan:
My plan for this and D11497 is:
- Get them in master.
- Some secret key / relatively-hidden preference activates the column.
- Quicksand activates //only// when the column is open.
- We can use column + quicksand for a long period of time (i.e., over the course of Conpherence v2 development) and hammer out the long tail of issues.
- When it derps up, you just hide the column and you're good to go.
Reviewers: btrahan, chad
Reviewed By: chad
Subscribers: epriestley
Maniphest Tasks: T2086, T7014
Differential Revision: https://secure.phabricator.com/D11507
Summary:
Fixes T7054. Fixes T7049.
- Stop scrolling Differential reticles when the page scrolls.
- Make dialogs aware of multi-panel UI.
Test Plan:
- Dialogs pop up in the right place.
- Inline + scroll now longer leaves the inline in a fixed position.
Reviewers: btrahan, chad
Reviewed By: chad
Subscribers: epriestley
Maniphest Tasks: T7049, T7054
Differential Revision: https://secure.phabricator.com/D11523
Summary:
When JX.Scrollbar activates, the page needs to be clicked before scrolling keys work.
Instead, set focus into the content after we set the page frame (if something else isn't already focused).
Also fixes T7042.
Test Plan: In Safari, Chrome and Firefox, scrolling with key commands is now immediately active.
Reviewers: btrahan, chad
Reviewed By: chad
Subscribers: epriestley
Maniphest Tasks: T7042
Differential Revision: https://secure.phabricator.com/D11508
Summary: See rPea67a8ab8e58. This was typed wrong and all the things I tested called scrollToPosition() directly.
Test Plan: Quoted a comment.
Reviewers: btrahan, chad
Reviewed By: chad
Subscribers: epriestley, krzysztof.ciebiera
Differential Revision: https://secure.phabricator.com/D11505
Summary: Fixes T7033. When we've reframed the main page content we need to scroll relative to the containing frame, not relative to the window.
Test Plan:
In Safari, Chrome and Firefox, used j/k/J/K keys to navigate diff content.
Tried some other scroll-based beahviors, like jump-to-anchors.
(It looks like the highlighting reticle got slightly derped a while ago, but it's still functional, so I didn't mess with it.)
Reviewers: btrahan, chad
Reviewed By: chad
Subscribers: epriestley
Maniphest Tasks: T7033
Differential Revision: https://secure.phabricator.com/D11490
Summary:
Fixes two issues:
- In Firefox, dragging outside the window and releasing the mouse button would miss the `mouseup` event. This would leave the bar dragging, even though the user had released the mouse button.
- In all browsers, dragging the handle and then holding your cursor in one place for more than a second would hide the handle. Instead, never hide the handle during a drag.
Test Plan:
- In Firefox, dragged handle right (outside of window) and released mouse button. Waved cursor over window; no more "sticky" scroll.
- In FF/Chrome/Safari, dragged handle and held cursor in same position for several seconds. No more handle hide.
- Waved cursor over window and made sure normal hiding still works.
Reviewers: btrahan, chad
Reviewed By: chad
Subscribers: epriestley
Differential Revision: https://secure.phabricator.com/D11487
Summary: See D11472. I eyeballed the "140" number by screenshotting / measuring in Paint.
Test Plan: Made the snapback thing return `true` and got snapback on OSX.
Reviewers: chad
Reviewed By: chad
Subscribers: avivey, epriestley
Differential Revision: https://secure.phabricator.com/D11485
Summary: In Safari, Firefox and Chrome drags outside the window will work if we do this. Safari didn't work before, not sure about the other two.
Test Plan: Clicked the scroll handle, then dragged my mouse to the right (outside the window) and down. Page scrolled in Safari, Firefox, and Chrome.
Reviewers: btrahan, chad
Reviewed By: chad
Subscribers: epriestley
Differential Revision: https://secure.phabricator.com/D11484
Summary:
See <rPc40bc0c8bf75#4050>. Repro steps:
- Scroll partway down the page.
- Click and drag the scroll handle.
Prior to this diff, the handle incorrectly jumps back to the top of the page. This is because we didn't store the handle's original position. (In testing, I always dragged from near the top of the page, and I don't normally drag scrollbars, so I didn't notice this.)
Test Plan: Clicking and dragging a partially scrolled handle now works correctly.
Reviewers: btrahan, chad
Reviewed By: chad
Subscribers: epriestley
Differential Revision: https://secure.phabricator.com/D11483
Summary:
See discussion on rPc40bc0c8bf75. Fixes a couple of glitchy things:
- Things were generally not nice on iOS.
- On OSX, with no mouse, the OS scrollbar and our fake scrollbar would both draw.
- Bar z-index was not set quite correctly.
Specifically, check if we need these bars. If we don't, just exit immediately and use the OS bars.
Test Plan:
- Tested Safari, Firefox, Chrome with and without a mouse.
- Tested iOS Simualtor.
Reviewers: btrahan, chad
Reviewed By: chad
Subscribers: epriestley
Differential Revision: https://secure.phabricator.com/D11480
Summary:
Ref T7014. With a mouse plugged in, multi-panel UIs are pretty hideous on OSX. This is somewhat offputting for me in Conpherence, and really jumps out at me with the new column mocks in T7014.
Sites like Twitch and Facebook approach this by emulating the touchpad scrollbar to achieve a more aesthetic UI. Use a similar approach.
This:
- Replaces the main scrollbar with a prettier fake one.
- This prepares the standard page frame for a persistent chat column.
Test Plan:
- Seems to work properly on OSX, Chrome and Firefox. Haven't tested on IE; my Windows setup is pretty iffy at the moment.
- Tried Conpherence.
- Tried Workboards.
Reviewers: btrahan, chad
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T7014
Differential Revision: https://secure.phabricator.com/D11472
Summary: This variable is used before it is defined.
Test Plan: `arc lint`
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11434
Summary: Enable strict mode for Javelin when running in NodeJS.
Test Plan: Made sure Aphlict still worked.
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11432
Summary:
This was broken in D11383. Basically, I had the `ws` module installed globally whilst testing, but the changes made do not work if the `ws` module is installed locally (i.e. in the `./support/aphlict/server/node_modules` directory). After poking around, it seems that this is due to the sandboxing that is done by `JX.require`.
A quick fix is to just //not// use `JX.require`, although you may have a better idea?
The error that is occurring is as follows:
```
<<< UNCAUGHT EXCEPTION! >>>
Error: Cannot find module 'ws'
at Function.Module._resolveFilename (module.js:338:15)
at Function.Module._load (module.js:280:25)
at Module.require (module.js:364:17)
at require (module.js:380:17)
at extra.require (/usr/src/phabricator/webroot/rsrc/externals/javelin/core/init_node.js:48:16)
at /usr/src/phabricator/support/aphlict/server/lib/AphlictClientServer.js:10:17
at Script.(anonymous function) [as runInNewContext] (vm.js:41:22)
at Object.JX.require (/usr/src/phabricator/webroot/rsrc/externals/javelin/core/init_node.js:58:6)
at Object.<anonymous> (/usr/src/phabricator/support/aphlict/server/aphlict_server.js:102:4)
at Module._compile (module.js:456:26)
>>> Server exited!
```
Test Plan: Now able to start the Aphlict server.
Reviewers: joshuaspence
Reviewed By: joshuaspence
Subscribers: Korvin, epriestley
Maniphest Tasks: T6987
Differential Revision: https://secure.phabricator.com/D11425
Summary: What do you think this is, PHP?
Test Plan: Eyeball it.
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11361
Summary:
Fixes T4656. Helps users with this naming convention, which is probably not super duper rare.
Users will need to make an edit to a project -or- run bin/search index "#project-tag" to make this actually work.
Test Plan: made a project "[T4656test]". Typed "t4" and project showed up!
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T4656
Differential Revision: https://secure.phabricator.com/D11302
Summary:
Ref T6559. Adds a "JX.Leader" primitive to replace the synchronization over Flash. This does a couple of things:
- Offers an "onBecomeLeader" event, which we can use to open the WebSocket.
- Offers an "onReceiveMessage" event, which we can use to dispatch notifications and handle requests to play sounds and send desktop notifications.
- Offers a "broadcast()" method, which we can use to send desktop notification and sound requests.
Test Plan:
Added some code like this:
```
if (!statics.leader) {
statics.leader = true;
JX.Leader.listen('onBecomeLeader', function() {
JX.log("This tab is now the leader.");
});
JX.Leader.listen('onReceiveBroadcast', function(message, is_leader) {
JX.log('[' + (is_leader ? 'As Leader' : 'Not Leader') + '] ' + message);
});
JX.Leader.start();
}
```
Then:
- Saw first tab open become leader reliably in Safari, Chrome, Firefox.
- Saw new tab become leader reliably when the first tab was closed.
- Saw broadcast() work as documented and deliver messages with correct leadership-flag and uniqueness.
Reviewers: joshuaspence, btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T6559
Differential Revision: https://secure.phabricator.com/D11235
Summary: Ref T6559. Wraps WebSocket in a reasonable driver class which does event dispatch, some state management, and handles automatic reconnect.
Test Plan: In Safari, Firefox and Chrome, connected to a websocket server and sent messages back and forth. Terminated and restarted server, saw automatic reconnects successfully reestablish a connection on all browsers.
Reviewers: btrahan, joshuaspence
Reviewed By: joshuaspence
Subscribers: epriestley
Maniphest Tasks: T6559
Differential Revision: https://secure.phabricator.com/D11252
Summary:
Fixes T5251. We don't recompute tokenizer metrics accurately after a paste event.
Listen for paste events and redraw the input.
Test Plan: Pasted long text into a tokenizer in Safari, Firefox and Chrome.
Reviewers: btrahan, chad
Reviewed By: chad
Subscribers: epriestley
Maniphest Tasks: T5251
Differential Revision: https://secure.phabricator.com/D10442
Summary:
Fixes T5727. Updates the regexes to split on '-'. Also changes the editor such that tokens are updated by the larger search process. (Note this means we update this data more often then we need to - for every project transaction.)
Users will need to make an edit to a project -or- run `bin/search index "#project-tag"` to make this actually work.
Test Plan: Made "Frontend-Engineering", "Engineering", and "Backend-Enginering". They all showed up in the typeahead!
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Maniphest Tasks: T5727
Differential Revision: https://secure.phabricator.com/D10247
Summary:
Ref T5240. This fixes dragging stuff and using the mousewheel or keyboard to scroll the window during the drag.
(It does not fix "dragging near the edge of the container should scroll it" yet.)
Test Plan: Dragged stuff around on task list and workboards in Safari, Firefox and Chrome. Used mousewheel and shift + mousewheel to scroll the document and containers during drag. Object remained under the cursor.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T5240
Differential Revision: https://secure.phabricator.com/D10186
Summary:
Ref T5366. Currently, enabling `notification.debug` fails to produce any messages unless developer mode is also enabled.
There's no reason we need to disable JX.log in production, and the byte size of the method is very small. Always provide a real JX.log implementation.
Test Plan: Saw notification debugging without needing to enable developer mode.
Reviewers: btrahan, joshuaspence, hach-que
Reviewed By: hach-que
Subscribers: epriestley
Maniphest Tasks: T5366
Differential Revision: https://secure.phabricator.com/D10104
Summary:
Ref T4420. Fixes T5473. Currently, when typeahead results get redrawn, you can lose your cursor position. A simple way to reproduce this is type "dif", select "Differential" using the arrow keys, then type "f". The selection will be lost.
Instead: store the old selection, then look for an item with the same name in the new set and select it. In effect, this preserves any focus selection.
Test Plan:
- Typed "dif".
- Typed "down arrow key" to select "Differential".
- Typed "f".
- "Differential" remained selected.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T5473, T4420
Differential Revision: https://secure.phabricator.com/D9900
Summary: I'm pretty sure that `@group` annotations are useless now... see D9855. Also fixed various other minor issues.
Test Plan: Eye-ball it.
Reviewers: #blessed_reviewers, epriestley, chad
Reviewed By: #blessed_reviewers, epriestley
Subscribers: epriestley, Korvin, hach-que
Differential Revision: https://secure.phabricator.com/D9859
Summary:
Fixes T5273. Two issues:
- If a JX.Resource callback throws, we keep running it. Instead, make sure it gets cleaned up before raising an exception.
- The countdown timer script doesn't recover gracefully if the node has been removed from the document by the time it runs. Instead, just bail if we can't find the countdown.
Test Plan: Dumped `{Cxyz}` into a preview and got a countdown.
Reviewers: yungsters
Reviewed By: yungsters
Subscribers: epriestley
Maniphest Tasks: T5273
Differential Revision: https://secure.phabricator.com/D9399
Summary: 71 new icons! Pied Piper!
Test Plan: tested new icons on UIExample. Perused a few other pages in Maniphest, Differential. No issues noted.
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D9125
Summary:
A few fixes for IE that makes this work in IE9+ and at least you can drag things and have it fail horribly in IE8 now. Ref T5023.
- try / catch around getBoundingClientRect because IE throws if the object is not attached to the DOM. The correct value can be inferred in these cases as 0,0
- turns out IE has different values for button here (1,2,4) than other browsers. special case this by using the other IE fact that 'which' will be null
Test Plan: dragged and dropped tasks successfully in IE9, IE10, and IE11 on Windows 7. dragged and dropped tasks unsuccessfully in IE8 on Windows 7 - they always "drop" to the 2nd position or so as I think the ondrag code isn't firing properly or something
Reviewers: chad, epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Maniphest Tasks: T5023
Differential Revision: https://secure.phabricator.com/D9118
Summary:
Allows you to quickly search for files within a repository. Roughly:
- We build a big tree of everything and ship it to the client.
- The client implements a bunch of Sublime-ish magic to find paths.
Test Plan: {F154007}
Reviewers: chad, btrahan
Reviewed By: btrahan
Subscribers: epriestley, zeeg
Differential Revision: https://secure.phabricator.com/D9087
Summary:
Fixes T430. Fixes T4834. Obsoletes D7641. Currently, we do some things less-well than we could:
- We just let the browser queue and prioritize requests, so if you load a revision with 50 changes and then click "Award Token", the action blocks until the changes load in most/all browsers. It would be better to prioritize this action and queue it immediately.
- Similarly, changes tend to load in order, even if the user has clicked to a specific file. When the user expresses a preference for a specific file, we should prioritize it.
- We show a spinning GIF when waiting on requests. This is appropriate for some types of reuqests, but distracting for others.
To fix this:
- Queue all (or, at least, most) requests into a new queue in JX.Router.
- JX.Router handles prioritizing the requests. Principally:
- You can submit a request with a specific priority (500 = general content loading, 1000 = default, 2000 = explicit user action) and JX.Router will get the higher stuff fired off sooner.
- You can name requests and then adjust their prorities later, if the user expresses an interest in specific results.
- Only use the spinner gif for "workflow" requests, which is bascially when the user clicked something and we're waiting on the server. I think it's useful and not-annoying in this case.
- Don't show any status for draft requests.
- For content requests, show a subtle hipster-style top loading bar.
Test Plan:
- Viewed a diff with 93 changes, and clicked award token.
- Prior to this patch, the action took many many seconds to resolve.
- After this patch, it resolves quickly.
- Viewed a diff with 93 changes and saw a pleasant subtle hipster-style loading bar.
- Viewed a diff with 93 changes and typed some draft text. Previews populated fairly quickly and there was no spinner.
- Viewed a diff with 93 changes and clicked something with workflow, saw a spinner after a moment.
- Viewed a diff with 93 changes and clicked a file in the table of contents near the end of the list.
- Prior to this patch, it took a long time to show up.
- After this patch, it loads directly.
Reviewers: chad, btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T430, T4834
Differential Revision: https://secure.phabricator.com/D8979
Summary:
Dropdown menus are entirely dynamic right now and use custom CSS. Begin rebuilding them to use ActionList CSS.
This introduces PHUIX components which are basically JS copy/pastes of the PHP PHUI components, just implemented in JS.
We have two other dropdowns: policy controls and one in Conpherence. I'll convert those, then implement D8966.
Test Plan: {F150418}
Reviewers: chad, btrahan
Reviewed By: btrahan
Subscribers: epriestley
Differential Revision: https://secure.phabricator.com/D8973
Summary: We were correctly invoked a didSyntheticSubmit event on the form, but nothing was listening to it. Re-jigger the workflow submit code a tad so an onsyntheticsubmit event handler can be written to fill this gap. Fixes T4669.
Test Plan: edited comments, submitting both via clicking the button and apple + enter LIKE A BOSS
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Maniphest Tasks: T4669
Differential Revision: https://secure.phabricator.com/D8961
Summary: For the time being, no need to have these in the repository.
Test Plan: Reload UIExamples, only see FontAwesome
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D8835
Summary:
This adds FontAwesome and attempts to make use as icons as consistent as possible. May require additional tweaks once we start using, but in practice this is pretty finished.
- Adds FontAwesome
- Adds additional transforms (rotates, spins)
- Adds additional colors
- Better scopes halflings and fontawesome
- Shares CSS between fonts for consistency
Test Plan:
Tested various browsers back to IE8, mobile.
{F146146}
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D8818
Summary: This adds in the Glyphicons Halflings Font/Iconset as an option for PHUIIconView along with a standard set of 10 colors. This will be a replacement for the standard action icon set in upcoming diffs, as well as obviously give us more flexibility, less KB, and less design resource time managing images.
Test Plan: UIExamples, Diviner
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D8798
Summary: Ref T4714, tooltips in fullscreen mode need special math, due to fixed position throwing off position of tooltips.
Test Plan: Create work board, create several tasks, create several columns, drag tasks among columns and within columns. Create a paste, check tooltips in comment box show and are positioned correctly. Fullscreen comment box. Verify tooltips still show and position correctly.
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: epriestley, Korvin
Maniphest Tasks: T4714
Differential Revision: https://secure.phabricator.com/D8762
Summary: Bad news @cpojer @tomo. IE8 doesn't like you.
Test Plan: Load a diff in IE8; see changes and don't get JS errors.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: epriestley, tomo, Korvin, cpojer
Differential Revision: https://secure.phabricator.com/D8728
Summary: Being bold. I think it is fine to do this in 2011.
Test Plan: Run the new specs.
Reviewers: mroch, tomo, epriestley
Reviewed By: epriestley
Subscribers: chad, epriestley, aran
Differential Revision: https://secure.phabricator.com/D835
Summary: Ref T4587. Add an option to automatically generate a keypair, associate the public key, and save the private key.
Test Plan: Generated some keypairs. Hit error conditions, etc.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: aran, epriestley
Maniphest Tasks: T4587
Differential Revision: https://secure.phabricator.com/D8513
Summary:
- Allow Celerity to map and serve WOFF files.
- Add Source Sans Pro, Source Sans Pro Bold, and the corresponding LICENSE.
- Add a `font-source-sans-pro` resource for the font.
Test Plan:
- Changed body `font-face` to `'Source Sans Pro'`.
- Added `require_celerity_resource('font-source-sans-pro')` in StandardPageView.
Works in Firefox/Chrome/Safari, at least:
{F123296}
{F123297}
{F123298}
Reviewers: btrahan, chad
Reviewed By: chad
CC: chad, aran
Differential Revision: https://secure.phabricator.com/D8430
Summary:
Ref T4324.
- Create `Listener` to represent listening clients.
- Create `ListenerList` to represent the current list of clients.
- Create `Logfile` to handle logging.
Test Plan: Clicked "Send Test Notification", verified logs, status and notifications all work correctly.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T4324
Differential Revision: https://secure.phabricator.com/D8256
Summary:
Ref T4324. The server code is probably going to get a fair amount more complicated, so allow it to load Javelin classes in a mostly-reasonable way.
This integration has a few warts, but should be good enough to let us manage complexity through the next iteration of the server.
(Mostly I just want the concicse Javelin mechanism for defining new classes.)
Version bump is just so I can figure stuff out if this creates any issues for users based on which version of things they're running.
Test Plan: Started server, posted some messages through it.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T4324
Differential Revision: https://secure.phabricator.com/D8253
Summary: Ref T4441. In D8250 I added code to drop results if they don't match
the current typeahead state, but with the OnDemand source we were incorrectly
comparing normalized values to denormalized values. Instead, pass and compare
raw values.
Auditors: btrahan, chad
Summary: Ref T4420. I think the border is good enough without this spinny thing, and it needs a lot of code.
Test Plan: Used typeahead.
Reviewers: chad, btrahan
Reviewed By: chad
CC: chad, aran
Maniphest Tasks: T4420
Differential Revision: https://secure.phabricator.com/D8249
Summary:
Ref T4420. After the changes to the tokenizer, I sometimes do this:
- Type something like "diff" into a project typeahead.
- Select "differential".
- A fraction of a second later, the typeahead pops back open.
This is because I selected the result from a partial query (like "diff" running against the "di" results) and then the full results of the "diff" query came back to the browser.
Instead, when showing results, require that the current state match the state that the results are for: don't show "dog" results if the tokenizer now reads "cat", for whatever reason.
Test Plan: Added a 1s delay to results, typed "a", then typed "m" and selected a result in less than a second. Prior to the patch, the tokenizer would pop back open with "am" results afterward. Now, it doesn't.
Reviewers: btrahan, chad
Reviewed By: chad
CC: aran
Maniphest Tasks: T4420
Differential Revision: https://secure.phabricator.com/D8250
Summary:
Ref T4420. When a result list contains both open and closed results, hide the closed results. I think this has a good chance of almost always working, and feeling very intuitive. It has a small chance of being a weird mess. It feels reasonable to me so far
The one bad case I can come up with here is that if you have results which shadow each other, like "Apples" (a closed project) and "Apples and Bananas" (an open project), it is impossible to get "Apples" in the result list, because "Apples and Bananas" will always shadow it. Let's wait for someone to hit this before we figure out how to deal with it.
Test Plan: Typed through open stuff to hit closed stuff.
Reviewers: chad, btrahan
Reviewed By: chad
CC: aran
Maniphest Tasks: T4420
Differential Revision: https://secure.phabricator.com/D8238
Summary:
Ref T4420. Fixes T3309. Two major UX issues here:
- When the user extends a query ("alin" -> "alinc"), we currently hide all the results, then show them again when the new results arrive. This makes the typeahead feel a bit flickery. Instead, show matching results, then add more results when everything arrives.
- When loading more results from ondemand sources, we currently do not give you any indication that things are loading. Instead:
- Show a loading GIF (this might need #design help, @chad).
- Slightly lighten the control border.
- I didn't want to do anything like actually add "loading" text because it would cause UI flicker in the 'extend a query' case and some other cases, but otherwise this design is totally made up.
Test Plan: Typed into tokenizers and extended queries, got a better-feeling UI.
Reviewers: chad, btrahan
Reviewed By: btrahan
CC: chad, aran
Maniphest Tasks: T3309, T4420
Differential Revision: https://secure.phabricator.com/D8233
Summary:
Ref T1279. The new dual-mode user/project tokenizers are a bit disorienting. Provide content type hints.
Very open to any suggestions here, most of this patch is just getting the right data in the right places. We can change things up pretty easily.
- I like the little icons in the tokens themselves, I think they look good and are useful.
- I'm less sold on the '(Project)' thing I did in the dropdown. We can easily make this richer if you have thoughts on it -- we could put icons in the left column maybe? Or right-justify the types?
- I made it always sort users above projects.
Test Plan: See screenshot.
Reviewers: chad, btrahan
Reviewed By: btrahan
CC: chad, aran, carl
Maniphest Tasks: T4420, T1279
Differential Revision: https://secure.phabricator.com/D7250
Summary:
The issue was noticed in Firefox when user with login "watch"
was registered in the database. And it was caused by Object
in Firefox having "watch" method.
Solved by checking for whether lookup object does have own
property before using it as a map key.
Test Plan:
To test the issue simply create a user with login "watch",
open the phabricator site in Firefox and try to assign any
maniphest task to this user. You'll see exception being
printed to the javascript console (in my case it's printed
to firebug console).
Reviewers: #blessed_reviewers, epriestley
Reviewed By: epriestley
CC: epriestley, aran
Differential Revision: https://secure.phabricator.com/D8037
Summary: Currently, when a dialog is submitted the Workflow itself emits an event but no DOM event is emitted. The workflow event is fine for handlers which only use JS, but there's currently no way for a handler to act more like a normal form handler. This event gives normal form handlers a way to capture Workflow submits and muck around with form contents, etc.
Test Plan: In a future diff, edited policies via a Workflow dialog.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D7295
Summary: See comment; see IRC. This didn't cause anything bad, but ended up with `document.body.id == "null"` in Firefox, which is a bit weird.
Test Plan: Loaded page, put `document.body.null` into the console, got consistent results.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D7288
Summary: Fixes T3853. See inline comments for details.
Test Plan: Using iOS simulator, mashed the right hand side of tokenizers.
Reviewers: chad, btrahan
Reviewed By: chad
CC: aran
Maniphest Tasks: T3853
Differential Revision: https://secure.phabricator.com/D7049
Summary: Fixes T3834. We have some hack code here for Firefox, but I can't reproduce the original Firefox issue in modern Firefox. It's better to break weird copy/paste edge cases than all Japanese input, in any case.
Test Plan: See screenshot.
Reviewers: chad, btrahan
Reviewed By: chad
CC: aran
Maniphest Tasks: T3834
Differential Revision: https://secure.phabricator.com/D7024
Summary:
Fixes T3814. Broadly, remarkup tables in inline comments did not work properly. I ran into several messes here and cleaned up some of them:
- Some of this code is doing `JX.$N('div', {}, JX.$H(response.markup))`, to turn an HTML response into a node, passing that around, and then doing junk with it. This is super old and gross.
- The slightly more modern pattern is `JX.$H(response.markup).getFragment().firstChild`, but this is kind of yuck too and not as safe as it could be.
- Introduce `JX.$H(response.markup).getNode()`, which actually expresses intent here. We have a bunch of `getFragment().firstChild` callsites which should switch to this, but I didn't clean those up yet because I don't want to test them all.
- Switch the `JX.$N('div', {}, JX.$H(response.markup))`-style callsites to `JX.$H(response.markup).getNode()`.
- `copyRows()` is too aggressive in finding `<tr />` tags. This actually causes the bug in T3814. We only want to find these tags at top level, not all tags. Don't copy `<tr />` tags which belong to some deeper table.
- Once this is fixed, there's another bug with mousing over the cells in tables in inline comments. We select the nearest `<td />`, but that's the cell in the remarkup table. Instead, select the correct `<td />`.
- At this point, these last two callsites were looking ugly. I provided `findAbove()` to clean them up.
Test Plan: Created, edited, deleted, moused over, and reloaded a revision with inline comments including remarkup tables. Used "Show more context" links.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T3814
Differential Revision: https://secure.phabricator.com/D6924
Summary: Ref T2715. When you type "T12", etc., into the search box, use ApplicationPHIDs to try to find an object name match.
Test Plan: Typed "T12", "rP", "Q11", etc.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2715
Differential Revision: https://secure.phabricator.com/D6618
Summary:
Currently, it takes one frame for animations to set their first value. For fading stuff in, that means it briefly appears at 100% opacity, then jumps to 0%, then fades in from there.
Instead, immediately tween to the initial value.
Test Plan: Comments in Pholio fade in nicely. Preview is still a janky pile of mess until D6267.
Reviewers: btrahan, chad
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D6268
Summary:
Fixes T1945. Ref T2947. At various times, installs (Disqus, Dropbox, etc.) have asked for a way to edit tasks more quickly. Provide edit-from-lists.
{F44700}
{F44701}
The one rough edge on this is that if you change the task priority we update it inline but don't move it. It's probably infeasible to actually move it, but maybe we could give it some sort of visual style to indicate that it's dirty.
Test Plan: Edited tasks normally and via this action thing.
Reviewers: chad, btrahan
Reviewed By: chad
CC: tido, deuresti, ahoffer, aran
Maniphest Tasks: T1945, T2947
Differential Revision: https://secure.phabricator.com/D6086
Summary: If `jsxmin` is not available, use a pure PHP implementation instead (JsShrink).
Test Plan:
- Ran `arc lint --lintall` on all JS and fixed every relevant warning.
- Forced minification on and browsed around the site using JS behaviors. Didn't hit anything problematic.
Reviewers: vrana, btrahan
Reviewed By: vrana
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5670
Summary: Fixes T3140. Previously, we added the highlight class only on explicit events, but not on actual focus events (e.g., via tab).
Test Plan: Tabbed into a tokenizer. Clicked a tokenizer. Tabbed out of a tokenizer. Clicked out of a tokenizer. Shift-tabbed out of a tokenizer. Verified highlight state was always correct.
Reviewers: chad
Reviewed By: chad
CC: aran
Maniphest Tasks: T3140
Differential Revision: https://secure.phabricator.com/D5891
Summary: Brings the Javelin change in D5832 into Phabricator.
Test Plan: Hit a `/?&x` URI without hanging Safari.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D5833
Summary:
General cleanup and separation into generic vs Stripe blocks of code.
- There was an old CC form view for Stripe stuff that I never cleaned up; clean that up.
- Move non-Stripe CC form rendering into a base class (Balanced can reuse it).
- Move non-Stripe CC form JS into a shareable class.
- Simplify JS a bit (JX.Workflow can add extra parameters to a request, so we don't need hidden inputs).
- Genericize CSS.
- Depend on Stripe JS directly, if they're down we're not going to be able to add cards anyway.
Ref T2787.
Test Plan: Hit all Stripe errors and added new cards.
Reviewers: btrahan, chad
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2787
Differential Revision: https://secure.phabricator.com/D5758
Summary: For some evil design purpose.
Test Plan: Focused, blurred tokenizers.
Reviewers: chad
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D5780
Summary:
this diff makes hovercards load and draw but a single time as appropos; pre-diff we could hammer the server by moving the mouse a bunch before the initial load and the re-draw event was continuously firing.
also makes hovercards work inside whacked out divs like the Conpherence message panel. Fixes T2949.
Test Plan: played with conpherence and phriction, observing hovercards showing up like they should
Reviewers: epriestley, chad
Reviewed By: epriestley
CC: AnhNhan, aran, Korvin
Maniphest Tasks: T2949
Differential Revision: https://secure.phabricator.com/D5719
Summary:
Currently, Celerity map rebuilds on Windows don't put Stripe or Raphael into the map. Move them into `webroot/rsrc/externals/` so they get picked up.
At some point we should maybe let the mapper load resources from mulitple locations, but this is more straightforward for now.
See https://github.com/facebook/phabricator/issues/294
Test Plan: Rebuilt map, verified Burnup Rate + Stripe work.
Reviewers: vrana, btrahan
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D5661
