Summary: Moving towards a consisent 'if header, show in object box' style around Phabricator.
Test Plan:
Grep for uses of RevisionList and make sure double boxes arent set, browse Differential, various searches, a revision, and a commit.
{F282113}
{F282114}
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11651
Summary: Minor, adds border, reduces greys, etc.
Test Plan:
View a number of config issues, see new colors.
{F282035}
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11650
Summary: Just makes the UI cleaner on full width or dialog forms (and mobile)
Test Plan:
run into a bunch of errors, test mobile breakpoints.
{F281585}
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11638
Summary: Fixes T7115, at least for me. Unclear if this is the "correct" fix.
Test Plan: Try to print, get page.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T7115
Differential Revision: https://secure.phabricator.com/D11636
Summary: Little more header spacing, more blu-ish inline with other elements.
Test Plan:
test a table before and after.
{F281094}
{F281095}
Reviewers: epriestley, btrahan
Reviewed By: btrahan
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11620
Summary: Revamps Profile to be like Projects, a mini portal and side nav with icons.
Test Plan: Viewed my own profile, as well as others. Test seeing my commits, tasks, diffs, and upcoming events. Checked mobile navigation.
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11547
Summary: Before the redesign, these levels were hidden. This recreates that behavior.
Test Plan:
Test nested Phriction pages
{F281114}
{F281115}
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11621
Summary: Right now you have to know to setFlush on an ObjectBox preceding a timeline, we can make that automatic with CSS.
Test Plan: Test some Diffusion commit pages, boxes now site flush under timeline. Check Maniphest and Differential, no changes.
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11617
Summary: Increase the CSS specificity on mobile so correct styles are applied.
Test Plan: Test a Maniphest Task
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11618
Summary: Clean up the error view styling.
Test Plan:
Tested as many as I could find, built additional tests in UIExamples
{F280452}
{F280453}
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: hach-que, Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11605
Summary: These should be consistent with ObjectBox headers.
Test Plan: Review a Phriction document
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11607
Summary: Minor tweaks to font size, message pane spacing.
Test Plan: use more common spacing (4px grid)
Reviewers: epriestley, btrahan
Reviewed By: btrahan
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11583
Summary: Main plan is to give conversations in Conpherence or Durable Column a different, lighter, chatty feel like Phriction.
Test Plan:
Tested a couple of threads and remarkup styles.
{F278086}
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11562
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: Fix 'No Conpherences' layout, add 'Recent' label to list.
Test Plan: test with and without a list of threads.
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11569
Summary:
Fixes T5039. The trick / possibly lame part here is we only match 1 application email and its undefined which one. e.g. if a user emails us at address x, y, and z only one of those will pick up the mail. Ergo, don't let users define non-sensical herald conditions like "matches all". Also document what I think was non-intuitive about the code with an inline comment; we have to return an array with just a phid from an object and out of context it feels very "what the...???"
Note this needs to be deployed to other applications still, but I think its okay to close T5039 aggressively here since its done from a user story perspective.
Test Plan: set up a herald rule to flag tasks created as blue via app email x. sent an email to x via `bin/mail receive-test` and verified the task had the blue flag
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T5039
Differential Revision: https://secure.phabricator.com/D11564
Summary: Fixes T7084. This doesn't use the same anchor logic as other applications.
Test Plan: `$245` lines now jump to line 245 on page load.
Reviewers: btrahan, chad
Reviewed By: chad
Subscribers: epriestley
Maniphest Tasks: T7084
Differential Revision: https://secure.phabricator.com/D11563
Summary: Update logo/icon to flat white. I beleive this is the last of the 'chrome' iconography.
Test Plan:
Internally debate in my head the correct path. Place up for review with the Phabricator Reviewer Gods.
{F278039}
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11561
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: Add a setBorder call to CrumbsView to be more deliberate when a border is drawn. Could not find any CSS hacks to set it conditionally CSS.
Test Plan: Browsed every application that called crumbs and make a design decision. Also fixed a few bad layouts.
Reviewers: btrahan, epriestley
Reviewed By: btrahan
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11533
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: Swaps out AphrontPanels for ObjectBoxes. I'd like to start reducing the floating object lists around the site for consistency. Also, these should provide more items above the fold.
Test Plan:
Test on my local homepage. Built a fake welcome.html too, though I think that's deprecated.
{F277020}
{F277021}
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11529
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: Things on mobile should be 8px gutter.
Test Plan: view in smaller browser windows
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11525
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:
Ref T7014. This is very rough and not hooked up to anything, but gets a couple of the layout pieces in place so we can (a) see that it looks like it'll kinda work; (b) look for problematic interactions and (c) you can fix my mangling of your design.
NOTE: Press "\" to toggle the column.
Test Plan:
Feels pretty good to me?
{F275722}
Reviewers: btrahan, chad
Reviewed By: chad
Subscribers: epriestley
Maniphest Tasks: T7014
Differential Revision: https://secure.phabricator.com/D11497
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: Removes the 1x application icons, and uses the fonticons instead. Feed was only known location.
Test Plan:
feed, dashboards, grep for use
{F275636}
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: hach-que, Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11496
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: This seems to improve behavior on iOS, good call.
Test Plan: Hard to be totally sure since my local install isn't set up with a real phone, but behavior seems better on iOS simulator.
Reviewers: chad
Reviewed By: chad
Subscribers: epriestley
Differential Revision: https://secure.phabricator.com/D11481
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: Fixes T5344. Essentially, we only make the AJAX request to `/notification/individual/` if we are the leader tab (i.e. only one tab will make this request). Once a response has been received from the server (containing the contents of the notification), we broadcast the message contents back to all other tabs for rendering.
Test Plan:
Opened two tabs on `/notification/status/` and clicked "Send Test Notification".
**Before**
```lang=bash, name=tail -f /var/log/phabricator-access.log | grep /notification/individual/
[Tue, 13 Jan 2015 20:10:37 +1100] 17033 phabricator 10.0.0.1 josh PhabricatorNotificationIndividualController - /notification/individual/-200 236036
[Tue, 13 Jan 2015 20:10:37 +1100] 17657 phabricator 10.0.0.1 josh PhabricatorNotificationIndividualController - /notification/individual/-200 24130
```
**After**
```lang=bash, name=tail -f /var/log/phabricator-access.log | grep /notification/individual/
[Tue, 13 Jan 2015 20:11:15 +1100] 17657 phabricator 10.0.0.1 josh PhabricatorNotificationIndividualController - /notification/individual/-200 180217
```
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T5344
Differential Revision: https://secure.phabricator.com/D11360
Summary: Just some housekeeping... mostly just removing some unused variables.
Test Plan: Checked that I was still about to receive notifications from `/notification/status/`.
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11398
Summary:
For block-level elements that have a margin-top or margin-bottom set
(generally to 12px), also reset the appropriate margin to 0 when
they're a first-child or last-child of their parents.
The change doesn't affect nested lists, their selector is more specific.
Test Plan:
Look at some comments or wiki documents that end with different
block elements, verify that the margins are pretty.
Reviewers: epriestley, #blessed_reviewers, chad
Reviewed By: #blessed_reviewers, chad
Subscribers: Korvin, epriestley
Projects: #remarkup
Maniphest Tasks: T6968
Differential Revision: https://secure.phabricator.com/D11382
Summary: Fixes T6964, makes action links float instead of absolutely positioned.
Test Plan: Tested UIExamples, actions in single line headers, multi line headers, headers with images, workboard headers. Test desktop, mobile, and tablet breakpoints. Long titles wrap as expected as button list grows.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T6964
Differential Revision: https://secure.phabricator.com/D11379
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:
A refresh of Projects including a new navigations UI.
- New Navigation UI.
- Auto switch default page if Workboard has been initialized
- Move Feed to it's own page
- Increase 'tasks' on Project Home to 50 over 10
- Fix various display bugs on Workboards
- Remove 'crumbs' from Project portal (unneeded).
Test Plan:
- clicked a link for a project with no workboard and saw the profile
- clicked a link for a project with a workboard and saw the workboard
- navigated around the various edit pages, inspecting links and making sure things linked back to the new profile uri
{F266460}
{F266461}
{F266462}
{F266463}
{F266464}
Reviewers: epriestley, btrahan
Reviewed By: epriestley, btrahan
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11272
Summary: Ref D11340, I missed the comments being to excited to land.
Test Plan: Shrink window to mobile view, click on action menu.
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11347
Summary: Ref T5752, moves mobile action menus to the object box instead of crumbs.
Test Plan: View action menus at tablet, desktop, and mobile break points. Verify clicking buttons works as expected opening menu.
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T5752
Differential Revision: https://secure.phabricator.com/D11340
Summary: Fixes T6102. Gets the from D10867 running in the tokenizer. This thing is pretty copy / pastey, but I guess that's okay?
Test Plan: looked at a project //tokenizer// and typed "project". since I have a million things with the word "project" in it, I was delighted to see the "project" project first in this project tokenizer.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T6102
Differential Revision: https://secure.phabricator.com/D11305
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:
Fixes T6559. No more flash, use Websockets. This is less aggressive than the earlier version, and retains more server logic.
- Support "wss".
- Make the client work.
- Remove "notification.user" entirely.
- Seems ok?
Test Plan:
In Safari, Firefox and Chrome, saw the browsers connect. Made a bunch of comments/updates and saw notifications.
Notable holes in the test plan:
- Haven't tested "wss" yet. I'll do this on secure.
- Notifications are //too fast// now, locally. I get them after I hit submit but before the page reloads.
- There are probably some other rough edges, this is a fairly big patch.
Reviewers: joshuaspence, btrahan
Reviewed By: joshuaspence, btrahan
Subscribers: fabe, btrahan, epriestley
Maniphest Tasks: T6713, T6559
Differential Revision: https://secure.phabricator.com/D11143
Summary:
Ref T6713. This isn't very clean, and primarily unblocks D11143.
After D11143, I have a reliable local race where I submit, get a notification immediately, then get a double update (form submission + notification-triggered update).
Instead, make the notification updates wait for form submissions.
This doesn't resolve the race completely. The notification updates don't block chat submission (only the other way around), so if you're really fast you can submit at the same time someone else sends chat and race. But this fixes the most glaring issue.
The overall structure here is still pretty shaky but I tried to improve things a little, at least.
Test Plan: Chatted with myself, saw 0 races instead of 100% races.
Reviewers: btrahan, joshuaspence
Reviewed By: joshuaspence
Subscribers: epriestley
Maniphest Tasks: T6713
Differential Revision: https://secure.phabricator.com/D11277
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 T6179. This makes the interaction where users remove a task from a workboard much more pleasant.
Test Plan: Loaded up workboard for "A Project". Edited tasks and if / when I removed "A Project" they disappeared on save.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T6179
Differential Revision: https://secure.phabricator.com/D11259
Summary: CLosed is a pretty important state and black tends to blend in a bit. This bumps to an alternate color to improve ability to scan and know state of objects.
Test Plan:
Review a number of closed objects. I will follow up with another diff on 'Archived' colors.
{F261895}
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11222
Summary: Increase font size and contrast of all Object Headers.
Test Plan:
Sample a few, find it easier to read. I've been using this locally for a while.
{F261868}
{F261869}
Reviewers: epriestley, btrahan
Reviewed By: btrahan
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11221
Summary: Adding a set of 16 icons that match the typeahead icons.
Test Plan: review in photoshop
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T6856
Differential Revision: https://secure.phabricator.com/D11183
Summary: Moves from 33% to 50% widths, allows more room for text on smaller screens. (Don't think this gets much use anyways). Fixes T6855
Test Plan: Reload page, shrink, still 50%
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T6855
Differential Revision: https://secure.phabricator.com/D11184
Summary: Fixes T6846, cleans up spacing, makes it look scary red.
Test Plan: Fake an exception, see new layout
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T6846
Differential Revision: https://secure.phabricator.com/D11161
Summary: If we have a byline, but no attributes, the height of the entire object is off by 4 pixels.
Test Plan: Review list of recent commits, see correct padding.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11128
Summary:
Fixes T6692. Addresses two main issues:
- The write guard would sometimes not get disposed of on exception pathways, generating an unnecessary secondary error which was just a symptom of the original root error.
- This was generally confusing and reduced the quality of reports we received because users would report the symptomatic error sometimes instead of the real error.
- Instead, reflow the handling so that we always dispose of the write guard if we create one.
- If we missed the Controller-level error page generation (normally, a nice page with full CSS, etc), we'd jump straight to Startup-level error page generation (very basic plain text).
- A large class of errors occur too early or too late to be handled by Controller-level pages, but many of these errors are not fundamental, and the plain text page is excessively severe.
- Provide a mid-level simple HTML error page for errors which can't get full CSS, but also aren't so fundamental that we have no recourse but plain text.
Test Plan:
Mid-level errors now produce an intentional-looking error page:
{F259885}
Verified that setup errors still render properly.
@chad, feel free to tweak the exception page -- I just did a rough pass on it. Like the setup error stuff, it doesn't have Celerity, so we can't use `{$colors}` and no other CSS will be loaded.
Reviewers: chad, btrahan
Reviewed By: btrahan
Subscribers: epriestley, chad
Maniphest Tasks: T6692
Differential Revision: https://secure.phabricator.com/D11126
Summary:
Ref T4411
I'm not quite sure if this is the right place for this as it will be difficult to provide proper user feedback of why we removed a particular subscriber.
Is the ApplicationTransactionEditor generally the right place to extract mentioned phids in comments?
On the other hand in some cases we cannot really give user feedback why a user was not subscribed (e.g.: commits & diffs)
Adding a diff to a repo where the user mentioned has no view permissions the subscriber is currently still added. Still would have to find where this is donet...
Any other places?
Unrelated: Is there any way to remove a subscriber from a commit/audit ?
Test Plan:
- Edited tasks with the mentioned user having view permissions to this specific task and without
- Raised concern with a commit and commented on the audit with the user having view permissions to the repo and without
- Added a commit to a repo with and without the mentioned user having permissions
- Mention a user in a task & commit comment with and without permissions
- Mentioning a user in a diff description & comments with and without permissions to the specific diff
Reviewers: chad, #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: chad, Korvin, epriestley
Maniphest Tasks: T4411
Differential Revision: https://secure.phabricator.com/D11049
Summary: This should be a local variable, not a global variable. This silences a few JSHint warnings.
Test Plan: `arc lint`
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11070
Summary: This variable should be local, not global. This silences a few JSHint warnings.
Test Plan: `arc lint`
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11068
Summary: This variable should be local, not global. This silences a bunch of JSHint warnings.
Test Plan: `arc lint`
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11067
Summary: This should be a fairly minor change that silences a bunch of JSHint warnings.
Test Plan: `arc lint` showed less warnings.
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11064
Summary:
The `show_details` function is used by DarkConsole. Adding this comment silences the following JSHint warning:
```
>>> Lint for webroot/rsrc/js/core/behavior-error-log.js:
Warning (W098) JSHintW098
'show_details' is defined but never used.
5
6 var current_details = null;
7
>>> 8 function show_details(row) {
9 var node = JX.$('row-details-' + row);
10
11 if (current_details !== null) {
```
Test Plan: `arc lint`
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11063
Summary: Currently, `editdata` is implicitly defined as a global variable (and JSHint complains about this). Instead, change it to be a local variable.
Test Plan: `arc lint` showed one less warning.
Reviewers: #blessed_reviewers, epriestley
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11062
Summary: Some CSS files are indented inconsistently, with a single space instead of two.
Test Plan: N/A
Reviewers: #blessed_reviewers, epriestley, chad
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11061
Summary: Fixes a few line length linter issues with CSS resources.
Test Plan: `arc lint`
Reviewers: #blessed_reviewers, epriestley, chad
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11060
Summary: If the image name in not defined, don't set it. Fixes T6793
Test Plan: View a lightboxed image, no 'undefined' alt tag
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T6793
Differential Revision: https://secure.phabricator.com/D11037
Summary: Removes the docs sprite in Conpherence with FontAwesome, adds additional icons. Unsure what happens if someone customized this config option.
Test Plan: Added images and files to a Conpherence, saw new icons.
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11028
Summary: These were converted to FontAwesome, but the images remained. Also removing a stray apps sprite (unused).
Test Plan: Visit button bar UIExample, Calendar. No issues. Grep codebase.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11026
Summary: Removes unused payments sprite and code, also some unused conpherence generated images. We use images in login (and could use FontAwesome, maybe).
Test Plan: grep codebase, pull up uiexamples icons page
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11025
Summary: These were refactored out a while ago
Test Plan: Grep codebase, use Conpherence on desktop, mobile.
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11023
Summary: Ref T6792, make blockquote text use darkblue, not blue.
Test Plan: Quote a bunch of text
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T6792
Differential Revision: https://secure.phabricator.com/D11018
Summary: The stacked version of property lists is supposed to display like a normal definition list.
Test Plan: Test the stacked layout in Herald.
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11014
Summary: We're using two different greys here.
Test Plan:
Inspect both elements, now serving the same grey.
{F253237}
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11009
Summary:
Ref T5833. This allows Almanac ServiceTypes to define default properties for a service, which show up in the UI and are more easily editable.
Overall, this makes it much easier to make structured/usable/consistent service records: you can check a checkbox that says "prevent new allocations" instead of needing to know the meaning of a key.
Test Plan: {F251593}
Reviewers: chad, btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T5833
Differential Revision: https://secure.phabricator.com/D10996
Summary: (some) international keyboard layouts can not type "~" in a way as to trigger this so use "@" instead. Save the suggested "+" as that seems like it would be useful for some future "adding stuff" keyboard workflow. Pretty stoked to get this squared away as I am quite confident our unreleased product will now be a huge smashing success. Ref T6683.
Test Plan: made sure my choice was okay via https://en.wikipedia.org/wiki/Dead_key#Dead_keys_on_various_keyboard_layouts; used the "@" key to show all transactions
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T6683
Differential Revision: https://secure.phabricator.com/D10983
Summary: Some browsers, mobile, didn't show 1px space between objects (due to font renderings). This enforces the size for consistent cross-browser display.
Test Plan: Test IE10
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D10970
Summary: I didn't get this quite right.
Test Plan:
- Clicked to open, saw white, then closed by:
- Clicking document outside menu;
- clicking menu icon again;
- clicking a different menu icon.
- In all three cases, got correct close + un-white behavior.
Reviewers: chad
Reviewed By: chad
Subscribers: epriestley
Differential Revision: https://secure.phabricator.com/D10967
Summary: I think this is what you're after?
Test Plan: clicky clicky
Reviewers: chad
Reviewed By: chad
Subscribers: epriestley
Differential Revision: https://secure.phabricator.com/D10966
Summary: Fixes T6683.
Test Plan: clicked the yellow box and it worked! pressed '~' and it worked!
Reviewers: epriestley, chad
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T6683
Differential Revision: https://secure.phabricator.com/D10932
Summary: Hides the second column of info on narrow (33%) columns on dashboards on object-items.
Test Plan:
Tested a narrow left and narrow right column, 160px is gained back.
{F248276}
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D10962
Summary: Ref T6723. Because of the sidenav, some dashboard layouts may perform poorly on more narrow desktop displays. This provides an additional media query and CSS rules.
Test Plan:
Move viewport to 1001 px, see desktop, move to 999 pixels, see tablet-like display.
{F248158}
{F248159}
{F248160}
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T6723
Differential Revision: https://secure.phabricator.com/D10956
Summary: Crumbs misalign because the hit area on the icons was 2px too large.
Test Plan: Test on pages with crumbs. IE, FF, Chrome...
Reviewers: epriestley, btrahan
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D10949
Summary: Cleans up spacing, updates to fonts instead of images. Fixed some mobile issues.
Test Plan:
Test with and without counts on desktop, tablet, mobile. Test layout in FF, Chrome, IE.
{F246745}
{F246746}
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D10948
Summary: This rule is making text actually more difficult to read on Windows, as well as is broken on Win/Chrome, and evidentally has performance issues on mobile browsers.
Test Plan: Turn it off, can still read web page
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D10940
Summary: Lots of minor spacing/alignment tweaks on mobile menus
Test Plan: set browser to 320 width, inspect for issues and alignments
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D10937
Summary: Change icon for Settings app to more match previous. Also align plus icon a little better.
Test Plan: Lots of staring.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D10934
Summary: we still need to be pager-sensitive, but otherwise this "show all" stuff is dead, dead dead...! Ref T4712. I think we can close the book on T4712 with one more diff to clean up the array_reverse / reverse paging stuff? That diff is probably a bit tricky as it involes auditing every TransactionQuery callsite...
Test Plan: viewed a task with a lot of transactions. clicked "show older" and it worked!
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T4712
Differential Revision: https://secure.phabricator.com/D10926
Summary: Fixes T6693.
Test Plan:
Made a bunch of comments on a diff with differential, being sure to leave inlines here and there. This reproduced the issue in T6693. With this patch this issue no longer reproduces!
Successfully "showed older changes" in Maniphest too.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T6693
Differential Revision: https://secure.phabricator.com/D10931
Summary: Updates header to use font-icons instead of images.
Test Plan: Test desktop and mobile layouts, Chrome, FF, Safari, IE.
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D10930
Summary: Ref T4712. This adds pagination. Future diffs will need to deploy `buildTransactionTimeline` everywhere and massage this stuff as necessary if we hit any special cases.
Test Plan: Set page size to "5" to make it need to paginate often. Verified proper transactions loaded in and the javascript actions worked.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T4712
Differential Revision: https://secure.phabricator.com/D10887
Summary: Converts PHUIObjectItemView to use display: table rows and columns for more flexible layouts. Slightly increases spacing, improves mobile layouts. Fixes T5502
Test Plan: Tested in multiple applications and UIExamples. Ran through mobile, tablet, and desktop break points. Used IE8-IE10, Firefox, Chrome, Safari on both Mac and Windows.
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T5502
Differential Revision: https://secure.phabricator.com/D10917
Summary: Fixes T6657. Some other rule got stronger and started overriding this one, I think. Removes bullets from checkbox lists.
Test Plan: Looked at a checkbox list.
Reviewers: btrahan, chad
Reviewed By: chad
Subscribers: epriestley
Maniphest Tasks: T6657
Differential Revision: https://secure.phabricator.com/D10912
Summary: Fixes positioning issues by creating another container to hold the abs. positioned arrows. (Issues primarily presented on Workboards).
Test Plan: Test UI arrows on a workboard, applciation launcher, and in UIExamples.
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D10897
Summary: Make captions feel less floaty, align with their input element.
Test Plan:
Visit a few forms with captions, test mobile and desktop layouts
{F236871}
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D10891
Summary: For actions like "Close" that are in theory stopping the timeline, we should display some disruption to the line itself.
Test Plan:
Tested in UIExamples
{F236077}
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D10884
Summary: Fixes T6102. Give "priority" treatment to strings that are exact matches.
Test Plan:
made a bunch of projects with the word project in them including "Project". before patch, "Project" wouldn't even show up if I typed "Project" - now its the second result right after the application "Projects".
{F235120}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T6102
Differential Revision: https://secure.phabricator.com/D10867
Summary: Fixes T4234 (Policy and Herald). I will have an additional diff for Maniphest Batch Editor, which looks jank in many other ways.
Test Plan:
Test editing a policy and some herald rules. Stuff lines up better.
{F235086}
{F235087}
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T4234
Differential Revision: https://secure.phabricator.com/D10864
Summary: Fixes T5015, Allow Herald rules for Maniphest to act on task status changes.
Test Plan: Create Herald rule for Maniphest tasks to flag a task with status "wontfix". Change status of Maniphest task to "wontfix". Task should be flagged.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T5015
Differential Revision: https://secure.phabricator.com/D10842
Summary: Right now, if no comment is selected the JS executes and throws an exception. Instead, if nothing is selected just do nothing. Fixes T6107.
Test Plan: opened up a commit in diffusion with an inline comment. pressed 'r' and saw no exceptions and nothing happen. pressed 'n' to select the next inline comment and then 'r' and it worked. opened up a commit in diffusion without any inline comments. pressed 'r' and saw no exceptions and nothing happen. opened up a diff in differential with an inline comment. pressed 'r' and saw no exceptons and nothing happened. pressed 'n' to select the next inline comment and then 'r' and it worked.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T6107
Differential Revision: https://secure.phabricator.com/D10843
Summary: Missed this in previous pass. Send these as links in HTML emails.
Test Plan: Register a new user that nees approval.
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D10815
Summary: I scoped this wrong and didn't test.
Test Plan: Hover on logo
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D10801
Summary: Ref T4214, this breaks the 'eye' out as a separate image 40px x 40px. We also now show the eye on mobile, as we have enough room for both currently.
Test Plan: Tested default and nightmaremoon colors, tested mobile, tablet and desktop layouts.
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T4214
Differential Revision: https://secure.phabricator.com/D10794
Summary: We're having trouble with this pointing at the wrong file after D10778; I assume that making it match the others is the proper fix...
Test Plan: Crossed fingers.
Reviewers: epriestley, #blessed_reviewers, chad
Reviewed By: #blessed_reviewers, chad
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D10785
Summary: Fixes T6469. Changes the default icon into text instead. Added the text to hidden boards and now display when reordering as well.
Test Plan:
Moved a bunch of columns, tested reordering. Seems more clear.
{F229626}
{F229627}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T6469
Differential Revision: https://secure.phabricator.com/D10784
Summary: Fixes T6452, provides actual href instead of just #
Test Plan: Control click and open link in new window, is correct task. Click and see item added to selection list.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T6452
Differential Revision: https://secure.phabricator.com/D10774
Summary: Fixes T6440. The issue is when a nested span appears, we don't apply the spacing.
Test Plan: Test a new diff in my sandbox that exhibits the issue. Ensure spacing now aligns.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T6440
Differential Revision: https://secure.phabricator.com/D10768
Summary:
I've been advised that that the MediaWiki flower needs to
appear with the square brackets for proper MediaWiki trademark
presentation.
See https://phabricator.wikimedia.org/T543 for relevant discussion.
Test Plan:
view login page with MediaWiki oauth plugin enabled,
see that the logo includes square brackets around the flower
Reviewers: epriestley, qgil, #blessed_reviewers, chad
Reviewed By: #blessed_reviewers, chad
Subscribers: Korvin, epriestley
Projects: #auth, #wikimedia
Differential Revision: https://secure.phabricator.com/D10752
Summary: I keep finding edge cases where this is causing issues, like PHUITagView in Audit as a property. Going to revert.
Test Plan: Review an Audit, tag is not cut off.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D10740
Summary: We removed other gradients, but not this one, for consistency.
Test Plan: UIExamples
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D10737
Summary: Fixes T6364. We should eventually remove the custom CSS from Maniphest, for now just share the styles.
Test Plan:
Add a dashboard panel to my home dashboard with GROUP BY set to Assigned on Maniphest Tasks.
{F222017}
Reviewers: epriestley, btrahan
Reviewed By: btrahan
Subscribers: hashar, Korvin, epriestley
Maniphest Tasks: T6364
Differential Revision: https://secure.phabricator.com/D10733
Summary: We recently shipping a more color correct version of Indigo, which means Pholio should use Pink for highlights.
Test Plan: Review Pholio Comments, Hovers.
Reviewers: epriestley, btrahan
Reviewed By: btrahan
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D10732
Summary:
Ref T5702. This is a forward-looking change which provides some very broad API improvements but does not implement them. In particular:
- Controllers no longer require `$request` to construct. This is mostly for T5702, directly, but simplifies things in general. Instead, we call `setRequest()` before using a controller. Only a small number of sites activate controllers, so this is less code overall, and more consistent with most constructors not having any parameters or effects.
- `$request` now offers `getURIData($key, ...)`. This is an alternate way of accessing `$data` which is currently only available on `willProcessRequest(array $data)`. Almost all controllers which implement this method do so in order to read one or two things out of the URI data. Instead, let them just read this data directly when processing the request.
- Introduce `handleRequest(AphrontRequest $request)` and deprecate (very softly) `processRequest()`. The majority of `processRequest()` calls begin `$request = $this->getRequest()`, which is avoided with the more practical signature.
- Provide `getViewer()` on `$request`, and a convenience `getViewer()` on `$controller`. This fixes `$viewer = $request->getUser();` into `$viewer = $request->getViewer();`, and converts the `$request + $viewer` two-liner into a single `$this->getViewer()`.
Test Plan:
- Browsed around in general.
- Hit special controllers (redirect, 404).
- Hit AuditList controller (uses new style).
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T5702
Differential Revision: https://secure.phabricator.com/D10698
Summary: Fixes T6318
Test Plan: Add tags to a Document in Phriction, verify line height with multiple tag icons.
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T6318
Differential Revision: https://secure.phabricator.com/D10719
Summary: Fixes issues seen in D10690 with unit results.
Test Plan: test D10690 and locally
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D10691
Summary: Ref T2787. Uses the real icons. Straightens out the add payment flow a tiny bit.
Test Plan: {F214922}
Reviewers: btrahan, chad
Reviewed By: chad
Subscribers: epriestley
Maniphest Tasks: T2787
Differential Revision: https://secure.phabricator.com/D10654
Summary: Fixes T6244, adds icons for payment providers. May split into different sprites down the road.
Test Plan: Photoshop
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T6244
Differential Revision: https://secure.phabricator.com/D10642
Summary: We should only move the footer over on desktops, where the sidenav is still present.
Test Plan: Test Mobile, Tablet, Desktop breakpoints on Herald.
Reviewers: epriestley, btrahan
Reviewed By: btrahan
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D10609
Summary: Removes nowrap when on mobile devices.
Test Plan: Test Lint/Unit layout on mobile and table layouts
Reviewers: epriestley, btrahan
Reviewed By: btrahan
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D10608
Summary: Fixes T6199, checks if Calendar is installed and displays if so.
Test Plan: Turned Calendar on and off, tested both layouts.
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Maniphest Tasks: T6199
Differential Revision: https://secure.phabricator.com/D10574
Summary:
Fixes T6084. Changes:
- Rename `phabricator.show-beta-applications` to `phabricator.show-prototypes`, to reinforce that these include early-development applications.
- Migrate the config setting.
- Add an explicit "no support" banner to the config page.
- Rename "Beta" to "Prototype" in the UI.
- Use "bomb" icon instead of "half star" icon.
- Document prototype applications in more detail.
- Explicitly document that we do not support these applications.
Test Plan:
- Ran migration.
- Resolved "obsolete config" issue.
- Viewed config setting.
- Browsed prototypes in Applications app.
- Viewed documentation.
Reviewers: chad, btrahan
Reviewed By: btrahan
Subscribers: epriestley, hach-que
Maniphest Tasks: T6084
Differential Revision: https://secure.phabricator.com/D10493
Summary:
Fixes T6044. We've had two cases (both the same install, coincidentally) where pages got hung doing too much data fetching.
When pages hang, we don't get a useful stack trace out of them, since nginx, php-fpm, or PHP eventually terminates things in a non-useful way without any diagnostic information.
The second time (the recent Macros issue) I was able to walk the install through removing limits on nginx, php-fpm, php, and eventually getting a profile by letting the page run for several minutes until the request completed. However, this install is exceptionally technically proficient and this was still a big pain for everyone, and this approach would not have worked if the page actually looped rather than just taking a long time.
Provide `debug.time-limit`, which should give us a better tool for reacting to this situation: by setting it to a small value (like 10), we'll kill the page after 10 seconds with a trace, before nginx/php-fpm/php/etc can kill it uselessly. Hopefully that will be enough information to find the issue (generally, getting a trace has been 95% of the problem in the two cases we've encountered).
Test Plan: Set this option to `3` and added a sleep loop, saw a termination after 3 seconds with a useful trace.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: csilvers, joshuaspence, epriestley
Maniphest Tasks: T6044
Differential Revision: https://secure.phabricator.com/D10465
Summary: Fixes T6052. Allow installs to link to legal documents, etc., in the page footer.
Test Plan:
- Configured a footer.
- Viewed workboards (no footer).
- Viewed Conpherence (no apparent disruption, I think everything z-indexes over the footer).
- Viewed stuff on mobile (seems OK).
- Viewed login page (saw footer).
{F201718}
Reviewers: btrahan, chad
Reviewed By: chad
Subscribers: epriestley
Maniphest Tasks: T6052
Differential Revision: https://secure.phabricator.com/D10466
Summary: Fixes T5368. Synchronizes the page title to reflect unread counts in the notification and Conphernece messages menus.
Test Plan: {F201083}
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T5368
Differential Revision: https://secure.phabricator.com/D10457
Summary:
Fixes T5979. There are three issues here:
- We cache document positions when you pick an item up, but don't recalculate them after you scroll, so they get out of date. Dirty the cache when the user scrolls.
- When we rebuild the cache during a drag (previously, this never happened), the position of the object you're dragging is computed wrong (since it has been moved to be under the cursor). Adjust the effective position of the object you've picked up to put it back in the right place in the list.
- When you fiddle around at the bottom of a column you can get jumpy redraws as the height adjusts. Put `min-height` on the container during a drag to prevent this.
Test Plan: In Safari, Chrome and Firefox, dragged items around on columns before and after scrolling the workboard panel.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T5979
Differential Revision: https://secure.phabricator.com/D10455
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:
Ref T4331. Ref T5968. Users sometimes have trouble figuring out how to ignore issues. The option is a bit hard to spot, especially if you aren't familiar with interfaces yet.
Make it a button on the issue page itself instead.
Test Plan:
Normal issue:
{F199225}
Ignored issue:
{F199226}
Fatal issue:
{F199227}
Reviewers: btrahan, chad
Reviewed By: chad
Subscribers: epriestley
Maniphest Tasks: T4331, T5968
Differential Revision: https://secure.phabricator.com/D10420
Summary:
Fixes T5979. When you drag a task and scroll using the mouse wheel, we don't move the task under your cursor until you move the mouse.
Instead, listen for `scroll` and move the task.
Test Plan:
- Clicked and dragged a task.
- While holding the task and not moving the cursor, used the mouse wheel to scroll.
- The task followed the cursor (previously, it stayed in position until the mouse was moved again).
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T5979
Differential Revision: https://secure.phabricator.com/D10416
Summary: Fixes T3913.
Test Plan: messed around with a comment on a commit and saw preview still updating correctly
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Maniphest Tasks: T3913
Differential Revision: https://secure.phabricator.com/D10382
Summary: D10355 made these a little too spacious.
Test Plan: {F195110}
Reviewers: chad, btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T4057
Differential Revision: https://secure.phabricator.com/D10360
Summary:
Fixes T4057. This sort of sidesteps the trickiest (but very rare) case of things like embedded slowvotes. We might be able to refine that later.
In the common bad case (macros, large images) it gets reasonable results by using `overflow: hidden` with `max-height`.
We use `PhabriatorMarkupEngine::summarize()` to try to just render the first paragraph.
Test Plan: {F195093}
Reviewers: chad, btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T4057
Differential Revision: https://secure.phabricator.com/D10355
Summary: Fixes T4769. This is silly and just scratches an itch, but do a better job with navigation sequences.
Test Plan: {F195082}
Reviewers: btrahan, chad
Reviewed By: chad
Subscribers: epriestley
Maniphest Tasks: T4769
Differential Revision: https://secure.phabricator.com/D10353
Summary: Fixes T5902, in theory float and inline-block achieve the same thing.
Test Plan: Test Safari, IE, and Chrome crumb layouts in Phriction, Diffusion, and Maniphest.
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Maniphest Tasks: T5902
Differential Revision: https://secure.phabricator.com/D10347
Summary: This should be auto, not scroll (which always shows bars).
Test Plan: test my diff in a few browsers
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D10302
Summary:
Ref T3725. This might eventually allow us to do `@username` typeaheads in textareas.
Javascript!!!
Test Plan:
Dumped this into console and got a "<<<" at the caret position in Safari, Firefox and Chrome.
```
setInterval(function() {
var area = JX.$('comment-content');
var r = JX.TextAreaUtils.getSelectionRange(area);
var d = JX.TextAreaUtils.getPixelDimensions(area, r.start, r.end);
JX.log(d);
try {
JX.DOM.remove(JX.$('ptr'));
} catch (_) {}
document.body.appendChild(
JX.$N(
'div',
{id: "ptr", style: { position: 'absolute', left: d.start.x + 'px', top: d.start.y + 'px', zIndex: 9999, border: '2px solid red' }},
'<<<'));
}, 1000);
```
Reviewers: chad, btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T3725
Differential Revision: https://secure.phabricator.com/D10280
Summary:
Fixes T5885. This implements optional soft point limits for workboard columns, per traditional Kanban.
- Allow columns to have a point limit set.
- When a column has a point limit, show it in the header.
- If a column has too many points in it, show the column and point count in red.
@chad, this could probably use some design tweaks. In particular:
- I changed the color of "hidden" columns to avoid confusion with "overfull" columns. We might be able to find a better color.
- UI hints for overfull columns might need adjustment.
(After T4427, we'll let you sum some custom field instead of total number of tasks, which is why this is called "points" rather than "number of tasks".)
Test Plan:
{F190914}
Note that:
- "Pre-planning" has a limit, so it shows "4/12".
- "Planning" has a limit and is overfull, so it shows "5 / 4".
- Other columns do not have limits.
- "Post-planning" is a hidden column. This might be too muted now.
Transactions:
{F190915}
Error messages / edit screen:
{F190916}
Reviewers: btrahan, chad
Reviewed By: btrahan
Subscribers: chad, epriestley
Maniphest Tasks: T5885
Differential Revision: https://secure.phabricator.com/D10276
Summary:
Ref T5885. See D10276.
Currently, ActionHeaders can only have minicons, and we don't use them anywhere and they probably don't make much sense in the product anymore.
Instead, allow them to have font icons. Remove minicons, which have no callsites and probably won't in the future.
Test Plan:
{F190925}
- Grepped for `minicons`.
- Grepped for `setHeaderIcon()`.
Reviewers: btrahan, chad
Reviewed By: chad
Subscribers: epriestley
Maniphest Tasks: T5885
Differential Revision: https://secure.phabricator.com/D10277
Summary: Ref T4427. This always counts 1 task = 1 point. The tricky bit is making this update in JS.
Test Plan: {F190900}
Reviewers: btrahan, chad
Reviewed By: chad
Subscribers: epriestley
Maniphest Tasks: T4427
Differential Revision: https://secure.phabricator.com/D10275
Summary: Fixes T5575. Moves "All" links into title/header. Mark all read floats left, and connection status sits in footer. Also added hints to enable notifications (it's a cool feature).
Test Plan:
Tested locally both menus.
{F190630}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Maniphest Tasks: T5575
Differential Revision: https://secure.phabricator.com/D10269
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: Fixes T5880.
Test Plan:
- Used event dialog in Calendar.
- Reviewed z-index.css history for likely conflicts, didn't see anything suspicious.
Reviewers: btrahan, chad
Reviewed By: chad
Subscribers: epriestley
Maniphest Tasks: T5880
Differential Revision: https://secure.phabricator.com/D10263
Summary: Fixes T5537. Makes it so I can scroll and see the bottom of notifications.
Test Plan: Open Notification Menu. Scroll. No longer follows me.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Maniphest Tasks: T5537
Differential Revision: https://secure.phabricator.com/D10261
Summary: Super important. This single icon unlocks untold riches and tens of corporate dollars.
Test Plan: Photoshop
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D10219
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:
Fixes T5677.
- Instead of using `sequence == 0` to mean "this is the backlog column", flag the column explicitly.
- Migrate existing sequence 0 columns to have the flag.
- Add the flag when initializing or copying a board.
- Remove special backlog logic when reordering columns.
Test Plan:
- Migrated columns, viewed some boards, they looked identical.
- Reordered the backlog column a bunch of times (first, last, middle, dragged other stuff around).
- Added tasks to a project, saw them show up in the reordered backlog.
- Initialized a new board and saw a backlog column show up.
- Copied an existing board and saw the backlog column come over.
- Tried to hide a backlog column.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T5677
Differential Revision: https://secure.phabricator.com/D10189
Summary: Just wanted to play with this, removes the gradient 'cards' for a flat design.
Test Plan:
Tested various apps, workboards
{F166127}
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: hach-que, epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D9515
Summary:
Ref T5024, T4427, T5474, T5523. Instead of separate icons in the column header for "Create Task" and "Edit Column Settings", use a dropdown menu.
- T5024 will likely add a "View Standalone" option.
- T4427 needs header space to show a count.
- T5474 likely needs "Edit Triggers..." (this seems reasonable to separate from editing the name, etc.)
- T5523 likely adds "Move all tasks..." eventually.
Test Plan: {F187414}
Reviewers: btrahan, chad
Reviewed By: chad
Subscribers: epriestley
Maniphest Tasks: T5523, T5474, T5024, T4427
Differential Revision: https://secure.phabricator.com/D10190
Summary:
Ref T4807. This doesn't actually do anything yet, but adds a dropdown menu for choosing an ordering and gets all the UI working correctly.
This also fixes a bug where column hidden state wouldn't persist across filter changes.
(I won't land this until it does something, but the next diff will probably be a mess so this seemed like a clean place to sever things.)
Test Plan:
{F187114}
- Altered sort ordering.
- Altered hidden state and filters, verified all states persisted correctly.
- Added `phlog()` to edit/create and move controllers and verified they receive sort information.
Reviewers: btrahan, chad
Reviewed By: chad
Subscribers: swisspol, chad, epriestley
Maniphest Tasks: T4807
Differential Revision: https://secure.phabricator.com/D10178
Summary: Use cutlery icon for hilarity. Ref T5768.
Test Plan: made something with remarkup in it, used 'view raw' and saw the remarkup raw in a nice little dialogue.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Maniphest Tasks: T5768
Differential Revision: https://secure.phabricator.com/D10183
Summary: This moves artifacts and build target messages into tabs.
Test Plan: Viewed build plan, saw the tabs appear when the steps had appropriate artifacts and / or messages.
Reviewers: #blessed_reviewers, epriestley, chad
Reviewed By: #blessed_reviewers, epriestley
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D10161
Summary: This automatically hides any empty build logs from Harbormaster, so that they do not appear.
Test Plan: Viewed a build plan where the logs were empty and didn't see them appear.
Reviewers: chad, #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D10091
Summary:
Fixes T4589. This implements much better policy behavior for files that aligns with user expectations.
Currently, all files have permissive visibility.
The new behavior is:
- Files uploaded via drag-and-drop to the home page or file upload page get permissive visibility, for ease of quickly sharing things like screenshots.
- Files uploaded via the manual file upload control get permissive visibility by default, but the user can select the policy they want at upload time in an explicit/obvious way.
- Files uploaded via drag-and-drop anywhere else (e.g., comments or Pholio) get restricted visibility (only the uploader).
- When the user applies a transaction to the object which uses the file, we attach the file to the object and punch a hole through the policies: if you can see the object, you can see the file.
- This rule requires things to use ApplicationTransactions, which is why this took so long to fix.
- The "attach stuff to the object" code has been in place for a long time and works correctly.
I'll land D8498 after this lands, too.
Test Plan:
- Uploaded via global homepage upload and file drag-and-drop upload, saw permissive visibility.
- Uploaded via comment area, saw restricted visibility.
- After commenting, verified links were established and the file became visible to users who could see the attached object.
- Verified Pholio (which is a bit of a special case) correctly attaches images.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T4589
Differential Revision: https://secure.phabricator.com/D10131
Summary: we were missing an e.kill(). Fixes T5754.
Test Plan: looked at a file and selected different tabs - pre-patch vertical position lost and post patch vertical position preserved!
Reviewers: chad, epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Maniphest Tasks: T5754
Differential Revision: https://secure.phabricator.com/D10111
Summary: Fixes T5188; see task for discussion.
Test Plan: made error condition occur with Firefox and was able to dismiss it by clicking 2x - once to focus window and then once to actually click in the window. Did this multiple times in a row with no errors.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Maniphest Tasks: T5188
Differential Revision: https://secure.phabricator.com/D10102
