Summary:
Fixes T4452. Ref T2009. There's a hierarchy of changeset rendering power: only low-level calls, use of ChangesetDetailView, then use of ChangesetListView (a list of DetailViews).
Prior to work here, the various changeset rendering controllers got their hands dirty to varying degrees, with some using only the lowest-level rendering pipeline:
- Phriction: no view (lowest level)
- Diffusion: DetailView
- Differential Changeset: DetailView
- Differential Diff: ListView
- Differential Revision: ListView
I brought Phriction up to use DetailView, but want to bring everything all the way up to use ListView. Each composition layer adds more features to diff browsing. In particular, this change enables "Highlight As", switching 1up vs 2up, adding inlines, etc., on the standalone view.
Test Plan:
- Viewed a changeset standalone. Could change highlighting, switch 1up vs 2up, add and edit inlines, etc.
- Viewed a revision; no behavioral changes.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T4452, T2009
Differential Revision: https://secure.phabricator.com/D12012
Summary: Fixes T7513. This is consistent with the behavior of an OS scrollbar.
Test Plan: Scrolled with haunting.
Reviewers: btrahan, chad
Reviewed By: chad
Subscribers: epriestley
Maniphest Tasks: T7513
Differential Revision: https://secure.phabricator.com/D12020
Summary:
Ref T2009. Currently, the code figures out if a comment is on the left or right by looking at the `<th />` preceeding the enclosing `<td />`.
This gets the right result in 2-up, but in 1-up rows are always `<th />`, `<th />`, `<td />`, so it always detects every inline as being in the new file.
Because "old" and "new" cells aren't inherently distingushable in the 1up view, we can't use a DOM test for this at all. Instead, just track this state explicitly.
Test Plan:
- Made left/right comments in 1up view and 2up view.
- Viewed them in 1up and 2up views.
- Hovered in 1up and 2up views.
- Diff-of-diff'd and reviewed old/new comments, then made some more.
Reviewers: btrahan, chad
Reviewed By: chad
Subscribers: epriestley
Maniphest Tasks: T2009
Differential Revision: https://secure.phabricator.com/D12011
Summary:
Ref T2009. Right now, when you mouse over a line number, we change the cursor to a "pointer", but that's the only hint we provide about the existence of inline comments.
Occasionally, users have reported confusion around how to leave inline comments.
Try to increase discoverability by showing the line reticle when you hover over the line.
(I could take this or leave it, but it seems OK / not annoying after 15 seconds of playing with it.)
Test Plan: Waved cursor over line numbers, attempted to test all the editor/noncontiguous/across-files cases.
Reviewers: btrahan, chad
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T2009
Differential Revision: https://secure.phabricator.com/D12010
Summary:
Ref T2009. Currently, the code which draws the reticle is sort of implicitly hard-coded with some of the rules for the 2up view.
Instead, use general rules:
- Start selection at the next `<td />`.
- End selection at the rightmost adjacent `<td />`.
These rules work in all cases.
Test Plan:
- Activated reticle in 1up and 2up views by clicking line numbers and hovering over comments. It now draws correctly.
- Dragged over line ranges in 1up and 2up views, saw accurate reticle.
Reviewers: btrahan, chad
Reviewed By: chad
Subscribers: epriestley
Maniphest Tasks: T2009
Differential Revision: https://secure.phabricator.com/D12009
Summary:
Ref T7014.
When no devices are connected, we don't meaningfully activate `JX.Scrollbar` (by design), but there was no fallback overflow CSS.
Also massage a couple of other CSS things to get sliiightly less broken behavior.
Test Plan:
- Disconnected USB devices; scrolled.
- Connected USB devices; scrolled.
Reviewers: btrahan, chad
Reviewed By: chad
Subscribers: epriestley
Maniphest Tasks: T7014
Differential Revision: https://secure.phabricator.com/D12008
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: Removes AphrontPanelView, and most of it's CSS - it seems some old previews still call it.
Test Plan: grep for AphrontPanelView, no callsites left. Verify CSS left is minimal needed.
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T7427
Differential Revision: https://secure.phabricator.com/D12004
Summary: Uses standard sidenav width, more spacing in labels, added background around textarea, make background work in Firefox.
Test Plan:
Test Desktop, Mobile, and Tablet break points. Test Firefox and Chrome.
{F331201}
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11993
Summary:
Ref T7014. This diff addresses
- getting it to be the right set of options
- add participant
- view in conpherence
- close window
- making those options work
- make it so if you are on /conpherence/ you can't toggle the durable column
Test Plan: inspected dom via chrome tools and found last transaction. added a participant and inspected the single new transactin added for accuracy. used view in conpherence action to view in conpherence. used close window action to close window
Reviewers: chad, epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T7014
Differential Revision: https://secure.phabricator.com/D11991
Summary:
Ref T2009. This reduces how buggy inlines are. They're still buggy.
Specifically, the inline endpoint didn't know how to scaffold inlines before, so some of them ended up rendering in the wrong rows or breaking layouts.
This passes the current renderer through to the inline editor endpoint, so it can at least get the layout correct.
Test Plan: Interacted with inlines in unified and side-by-side views.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T2009
Differential Revision: https://secure.phabricator.com/D11988
Summary:
These aren't being populated yet; they mostly fix some JS errors with inlines.
For example, the inline hover reticle relies on adjusting its width to account for the "copy" column, and failed when the column did not exist.
Test Plan:
- Hovering inlines in unified now works, mostly.
- Interacted with inlines in side-by-side.
Reviewers: chad, btrahan
Reviewed By: btrahan
Subscribers: epriestley
Differential Revision: https://secure.phabricator.com/D11985
Summary: Ref T2009. It doesn't make sense to have these as separate behaviors. We require a ChangesetViewManager to track view parameter state.
Test Plan: Interacted with changesets in Phriction, Differential and Diffusion.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T2009
Differential Revision: https://secure.phabricator.com/D11979
Summary:
Ref T2009. Currently, we do not persist view parameters when making context rendering requests.
The big one is the renderer (1up vs 2up). This makes context on unified diffs come in with too many columns.
However, it impacts other parameters too. For example, at HEAD, if you change highlighting to "rainbow" and then load more context, the context uses the original highlighter instead of the rainbow highlighter.
This moves context loads into ChangesetViewManager, which maintains view parameters and can provide them correctly.
- This removes "ref"; it is no longer required, as the ChangesetViewManager tracks it.
- This removes URI management from `behavior-show-more`; it is no longer required, since the ChangesetViewManager knows how to render.
- This removes "whitespace" since this is handled properly by the view manager.
Test Plan:
- Used "Show Top" / "Show All" / "Show Bottom" in 1-up and 2-up views.
- Changed file highlighting to rainbow, loaded stuff, saw rainbow stick.
- Used "Show Entire File" in 1-up and 2-up views.
- Saw loading chrome.
- No loading chrome normally.
- Made inlines, verified `copyRows()` code runs.
- Poked around Diffusion -- it is missing some parameter handling, but works OK.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T2009
Differential Revision: https://secure.phabricator.com/D11977
Summary: Ref T2009. This doesn't work when it's not on <body>, at least in modern iOS.
Test Plan:
Before:
{F330031}
After:
{F330033}
Reviewers: btrahan, chad
Reviewed By: chad
Subscribers: epriestley
Maniphest Tasks: T2009
Differential Revision: https://secure.phabricator.com/D11975
Summary: Ref T2009. Remove forced min-width of 780px in 1-up mode, and tweak a few other things to look better.
Test Plan: Looks better on mobile.
Reviewers: btrahan, chad
Reviewed By: chad
Subscribers: epriestley
Maniphest Tasks: T2009
Differential Revision: https://secure.phabricator.com/D11974
Summary: Ref T2009. These aren't good enough to actually use so I won't land this yet, but it makes testing changes a lot easier.
Test Plan:
- Swapped setting.
- Loaded revisions.
- Saw setting respected.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T2009
Differential Revision: https://secure.phabricator.com/D11972
Summary:
Ref T7014. This hooks up the durable column such that when you open it up it loads your most recent Conpherence. You can then switch amongst the various widgets and stuff and everything works nicely.
Except...
- scroll bar does not work
- also doesn't work at HEAD when I add a ton of text to the UI with no changes? (wrapped $copy in array_fill(0, 1000, $copy))
- "widget selector" does not collapse when you select something else
- this part wasn't really specified so I used the aphlict dropdown stuff. didn't want to keep working on that if this was the wrong UI choice
- can not edit title
- do we still want that to be done by clicking on the title, which pops a dialogue?
- can not add participants or calendar events
- what should this UI be? maybe just a button on the top for "participants" and a button on the bottom for calendar? both on top?
- this is not pixel perfect to the mock or two I've seen around. Aside from generally being bad at that, I definitely didn't get the name + timestamps formatting correctly, because the standard DOM of that has timestamp FIRST which appears second due to a "float right". Seemed like a lot of special-casing for what might not even be that important in the UI so I punted. (And again, there's likely many unknown ways in which this isn't pixel perfect)
There's also code quality issues
- `ConpherenceWidgetConfigConstants` is hopefully temporary or at least gets more sleek as we keep progressing here
- copied some CSS from main Conpherence app
- DOM structure is pretty different
- there's some minor CSS tweaks too given the different width (not to mention the DOM structure being different)
- copied some JS from behavior-pontificate.js to sync threads relative to aphlict updates
- JS in general is like a better version of existing JS; these should collapse I'd hope?
- maybe the aphlict-behavior-dropdown change was badsauce?
...but all that said, this definitely feels really nice and I feel like adding stuff is going to be really easy compared to how normal Conpherence is.
Also includes a bonus bug fix - we now correctly update participation. The user would encounter this issue if they were in a conpherence that got some updates and then they went to a different page; they would have unread status for the messages that were ajax'd in. This patch fixes that by making sure we mark participation up to date with the proper transaction in all cases.
Test Plan: hit "\" to invoke the column and saw nice loading UI and my latest conpherence load. sent messages and verified they received A-OK by looking in DOM console. toggled various widges and verified they rendered correctly. opened up a second browser with a second user on the thread, sent a message, and it was received in a nice asynchronous fashion
Reviewers: chad, epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T7014
Differential Revision: https://secure.phabricator.com/D11968
Summary: This moves global search results to use standard UI, and hopefully allow us to easily add more information.
Test Plan:
Tested a number of open and closed task queries, tried a few users and projects. All seem to work well.
{F328075}
{F328078}
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11948
Summary: Somewhat easier to parse and present information, with ICONS.
Test Plan:
Rebuilt current view with new layout. Tested toggling on and off some of the entries.
{F327816}
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11938
Summary:
Fixes T7424. Ref T6308.
Currently, there's no option to just add a card directly from the autopay UI. Add a button so this works.
Also, chip away at T6308 a bit. This isn't perfect but looks a little less out of place.
Test Plan:
{F327637}
- Added a payment method, then set it as autopay.
Reviewers: chad, btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T6308, T7424
Differential Revision: https://secure.phabricator.com/D11935
Summary: This filename is wrong ("phame" should be "passphrase").
Test Plan: Read filename.
Reviewers: chad, btrahan
Reviewed By: btrahan
Subscribers: epriestley
Differential Revision: https://secure.phabricator.com/D11934
Summary: Fixes some UIExample UI issues, adds a new full-width setting for DocumentView
Test Plan:
Test UIExamples at desktop and mobile breakpoints
{F327446}
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T7431
Differential Revision: https://secure.phabricator.com/D11933
Summary: Adding better CSS and set correct tag and examples.
Test Plan: Test UIExamples, creating and click on similar task, empty task in Maniphest.
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T7423
Differential Revision: https://secure.phabricator.com/D11932
Summary: Removes AphrontContext bar and uses PHUIInfoView instead. This also attaches to the ObjectBox instead for cleaner UI. Also moved phui-error-view.css which was missed.
Test Plan: Test creating a subtask or a new task, see updated info bar and action buttons.
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11920
Summary: Since this element isn't strictly about errors, re-label as info view instead.
Test Plan: Grepped for all callsites, tested UIExamples and a few other random pages.
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: hach-que, Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11867
Summary: Adds a new header color.
Test Plan:
Select new header color, see new header color.
{F326502}
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11918
Summary: This was missed in a recent rename.
Test Plan: No more console exception.
Reviewers: btrahan, chad
Reviewed By: chad
Subscribers: epriestley
Differential Revision: https://secure.phabricator.com/D11916
Summary: This diff moves the default monospace font from a Global Default config value to CSS. What this will allow is some flexibility in changing this font in other areas (like Diviner and DocumentView) without changing the defaults globally. However if the admin sets a config value or a user sets a config value, that value will trump all settings in the CSS files with an !important declaration in the page head.
Test Plan:
Currently tested:
- Setting no value
- Setting an admin value
- Setting a user value
Verify remarkup blocks in Differential, Diviner, Conpherence, and Diffusion look as expected.
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11597
Summary: Removes the special background and fonts. Uses just a simple bold header.
Test Plan:
Checked out installation guide and remarkup guide in Diviner. Looks cleaner.
{F314679}
{F314680}
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T6904
Differential Revision: https://secure.phabricator.com/D11869
Summary: Feed currently returns nothing is there are no stories, we can present a better view here by allowing a base and customizable set of errors. Fixes T7383
Test Plan:
Test a Project feed with no noDataSting and People with a noDataSting
{F321700}
{F321701}
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T7383
Differential Revision: https://secure.phabricator.com/D11897
Summary: This still needs some fine tuning, but wanted to get opinions. Using it on a laptop feels pretty good. This also moves `durable-column.css` into its own file since it'll likely continue to grow. Minor CSS tweaks to the near perfect rendition of durable column from pixel based mockups.
Test Plan:
Press \ on my laptop. Having issues with Chrome however, but FF and Safari work as expected.
{F322506}
Reviewers: epriestley, btrahan
Reviewed By: btrahan
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11901
Summary: Cleans up the UI a bit, removes excess spacing on ObjectList and adds space around history buttons.
Test Plan: Test a few changes in Phriction. Click on buttons.
Reviewers: epriestley, btrahan
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11904
Summary: Consolidate colors / spacing.
Test Plan:
Test embedding a paste, a list of pastes, and a PasteView for new colors, space.
{F321622}
{F321623}
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11896
Summary: Fixes T7382, specifically we were drawing double navs, removed those from each page and added the correct CSS rule.
Test Plan: Test a number of people and profile pages with a footer set.
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T7382
Differential Revision: https://secure.phabricator.com/D11895
Summary: For consistency, we switch back to base font in a few places when using alternate fonts like source-sans or monospace, this makes sure the base font is consistently reset.
Test Plan: Review a Document, a Diff, and a Legalpad form
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11891
Summary: Fixes T7135. Also does a bit of a javascript cleanup in that we had an event - "conpherence-selectthread" - which really didn't need to be an event.
Test Plan: selected various conpherences from the list and they loaded correctly, including putting the cursor at the end of the text as appropriate. send many messages rapid fire without ever taking my hands off the keyboard.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T7135
Differential Revision: https://secure.phabricator.com/D11890
Summary: This is the simplest fix I could find.
Test Plan: Scroll over sidenav on home, maniphest, workboards, etc. Test mobile, deskop. Firefox, Safari, Chrome, Internet Explorer.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11881
Summary: Fixes T7099, also picked some new colors. Raphael can bind the graph to a dom element, which resolved the scrolling issue.
Test Plan: Tested scrolling on my laptop, desktop. Seems resolved.
Reviewers: epriestley, btrahan
Reviewed By: btrahan
Subscribers: Korvin, epriestley
Maniphest Tasks: T7099
Differential Revision: https://secure.phabricator.com/D11879
Summary: I'm looking at beefing up PHUIErrorView for additional use cases as I remove some older AphrontViews. This will likely morph into PHUIInfoView and be a more lightweight version of PHUIObjectBox.
Test Plan:
UIExamples, mobile and desktop layouts. Have actual use cases coming in next diffs (may tweak design more then)
{F311943}
Reviewers: epriestley, btrahan
Reviewed By: btrahan
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11849
Summary: On /differential/ we have flush lists inside an object box which on mobile had unwanted padding. This padding shouldn't be set anymore.
Test Plan: Reviewed mobile home, differential, maniphest, applications, and uiexamples, noticed no artifacts to removal.
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11858
Summary: When a NOTE is at the top or bottom of a document, there is extra unwanted space.
Test Plan:
Write a NOTE as the top of a Phriction document
{F311233}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11846
Summary: We should only be adding space then a status is set, not a state.
Test Plan: Test on UIExamples, still renders as expected.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11844
Summary: Swaps out AphrontMiniPanelView usage with PHUIErrorView. Only used on homepage.
Test Plan:
Grepped for usage, only home. Revisit a new home, see modern componant.
{F310934}
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11842
Summary: The sky blue colors in the alerts was problematic in other color headers (not black). Rather than hand tweak each color, just going with white seems best. There is also a small animation now, which you may or may not like. It is playful and enjoyable to me at least.
Test Plan: Tested various header colors with and without alert notifications.
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11838
Summary: This adds Phacility "Indigo" as well as uses rgba for colors and hovers improving icon colors.
Test Plan:
Tested each color and hover states. Set test instance to scootaloo.
{F310660}
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11836
Summary: This increases the transparent space around the Phabricator logo. The logo itself is the same size. This allows for adding of other logos more easily without needing to alter the space provided. (Like Phacility)
Test Plan:
Reload page, screenshot logo, pull into Photoshop and verify spacing top and bottom.
{F309985}
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11828
Summary: Helvetica on modern Windows systems doesn't display very well, bolds are fairly not bold and leading between characters is all over the place. This change picks Segoe as the default Windows font which is easier to read, and the bolds are much crisper. There is even a slight improvement in text density with the improved leading.
Test Plan:
I've been using this locally for a few months, overall it's fairly similiar to Source Sans which people seem to enjoy.
{F309851}
{F309852}
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: hach-que, Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11825
Summary: Cleans up spacing, hides footer if nothing present, uses common colors.
Test Plan:
Write some typical for a designer code.
{F309840}
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11824
Summary: Uses PHUIObjectBoxView to display lists of diffs in Differential and Diffusion, unless embedded on a dashboard.
Test Plan:
Test Dashboard panel, Differential home, Commit, and Diff
{F282173}
{F282174}
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11659
Summary: We had two different blues for links here, cleaning that up.
Test Plan: Read a few Diviner documents
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11818
Summary: If we don't have a state in PHUIActionPanelView, don't set the extra padding to display it.
Test Plan: Review in UIExamples.
Reviewers: epriestley, btrahan
Reviewed By: btrahan
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11814
Summary: This change wraps the icon inline with the text, so smaller width icons have equal spacing between the border and text.
Test Plan:
review a number of different tag with icons, also UIExamples.
{F309048}
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11802
Summary: Sets everything up flush, I think this is the only indented block left.
Test Plan:
Write some code.
{F289438}
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley, #design
Differential Revision: https://secure.phabricator.com/D11721
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: Removes odd margin. Also removes min-height on content.
Test Plan: Test a fresh Phriction, and a deleted page.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11775
Summary: We use font-smoothing to make better looking buttons, but it makes text buttons look bad.
Test Plan: Test a button in an actionlist, font looks same as other action list items.
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11776
Summary: I feel like this got derped by recent updates, who knows.
Test Plan:
Tested a failed Project search in Projects and in Dashboards. Spacing seems correct
{F297441}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11755
Summary: Minor spring cleaning, improve the visual feel of the comments table, more consistent structure.
Test Plan:
Test multiple comments, long comments, short comments, and multiple lines.
{F282462}
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: hach-que, Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11666
Summary: Adds option for setting large text instead of icons. Adds success state.
Test Plan:
Built some more examples.
{F286388}
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11710
Summary: Super duper sized panels for singluar actions.
Test Plan:
UIExamples, will need more testing in Phacility.
{F286098}
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11709
Summary: Uses more standard boxes for display, and icons!
Test Plan:
Test with all enabled, all disabled, and a mix.
{F285945}
{F285946}
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11707
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
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: Fixes T5663. Said task kind of stalled out and maybe @chad wanted to take a different approach, but this works? Also, I think fixing any new bugs that may emerge with more reasonable css scoping seems like a reasonable path forward.
Test Plan: inline display of status icon looked correct in differential
Reviewers: epriestley, chad
Reviewed By: chad
Subscribers: epriestley, chad, Korvin
Maniphest Tasks: T5663
Differential Revision: https://secure.phabricator.com/D10084
Summary:
Depends on D9806. This implements the build simulator, which is used to calculate the order of build steps in the plan editor. This includes a migration script to convert existing plans from sequential based to dependency based, and then drops the sequence column.
Because build plans are now dependency based, the grippable and re-order behaviour has been removed.
Test Plan: Tested the migration, saw the dependencies appear correctly.
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D9847
Summary: At least on my install, sorting was pretty borked from a type issue. (e.g. "unbreak now" of 100 sorting as less than "High" of 90). Fix this with some parseInt action. Also support adding new cards with the new colsort stuff. The clever bit here is to include the task ID in the sorting vector because the task ID wins ties at the moment I think / new tasks need to show up before older tasks when they are initially created. Fixes T5716.
Test Plan: added many "normal" priority cards and saw them fly in correctly. changed priority and moved correctly. made no edits and no moves were made correctly.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Maniphest Tasks: T5716
Differential Revision: https://secure.phabricator.com/D10081
Summary: Ref T4896. Depends on D10055. This uses core rendering stuff for audit comments, and fixes all the wonkiness with inlines so we can actually land the migration.
Test Plan: Viewed, previewed and edited various types of comments in Diffusion.
Reviewers: chad, btrahan, joshuaspence
Reviewed By: joshuaspence
Subscribers: epriestley
Maniphest Tasks: T4896
Differential Revision: https://secure.phabricator.com/D10056
Summary: Adds Phriction to list of apps that use Source Sans as default font in addition to Legalpad and Diviner.
Test Plan: Tested various layouts imported from secure. Should be reasonably tested, but will follow up on secure.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D10064
Summary: Fixes T5719, Not completely convinced we won't see another ticket here, but overall testing it feels better.
Test Plan: dragon drop a lot of stuff.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Maniphest Tasks: T5719
Differential Revision: https://secure.phabricator.com/D10062
Summary: Ref T2787. There were some mega-uggo buttons and such; reduce the uggo-ness by a hair.
Test Plan: {F179686}
Reviewers: btrahan, chad
Reviewed By: chad
Subscribers: epriestley
Maniphest Tasks: T2787
Differential Revision: https://secure.phabricator.com/D10006
Summary: Fixes T5659. When building a token after a user selection, we currently use the `value` as the token text, but sometimes that's an internal name which doesn't make much sense to users. For projects, it is now "sluga slugb Proper Display Name". If available, use `displayName` instead.
Test Plan: Typed some projects into a tokenizer, got display names only.
Reviewers: chad, joshuaspence
Reviewed By: joshuaspence
Subscribers: epriestley
Maniphest Tasks: T5659
Differential Revision: https://secure.phabricator.com/D9996
Summary:
Ref T4420. This doesn't share all the code it really should, and renders a little odd. Make it more standard.
(Icons aren't handled totally correctly but there's no usability impact and all that code should just get cleaned up.)
Test Plan: Used custom policy typeahead, had a more standard experience.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T4420
Differential Revision: https://secure.phabricator.com/D9902
Summary: Ref T4420. We don't currently pass placeholder text properly, but should.
Test Plan: Saw placeholder text in Herald.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T4420
Differential Revision: https://secure.phabricator.com/D9901
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:
Ref T4420. Fixes T5306. Currently, the main menubar search has a lot of redundant/unshared code.
Move some common functions into `JX.Prefab.whatever()` and call them from the main search.
The major change here is that we apply the same "only show closed/disabled/archived objects if there are no matching open objects" logic, fixing T5306.
Test Plan:
- Used normal typeaheads.
- Used global search.
- Searched for a prefix shared by open and archived projects, didn't see the archived ones until the open ones were exhausted.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T5306, T4420
Differential Revision: https://secure.phabricator.com/D9899
Summary:
Ref T2787. Update some of the UI elements used by Phortune. Mostly gets rid of the old blue headers.
Also adds some sweet art.
Test Plan: Poked aroudn Phortune.
Reviewers: btrahan, chad
Reviewed By: chad
Subscribers: epriestley
Maniphest Tasks: T2787
Differential Revision: https://secure.phabricator.com/D9915
Summary: Fixes T4567. This isn't going to win design awards and we have some leaky CSS, but it works fine.
Test Plan: {F176743}
Reviewers: btrahan, chad
Reviewed By: chad
Subscribers: epriestley
Maniphest Tasks: T4567
Differential Revision: https://secure.phabricator.com/D9905
Summary:
Ref T5476. Currently, the task edit code assumes it knows what the UI looks like and sends back where on the column an item should be inserted.
This is buggy after adding filters, and relatively complex. Instead, send down the ordering on the whole column and sort it in the UI. This is a bit simpler overall and more general. It makes it easier to further generalize this code for T5476.
Test Plan:
- Edited a task on a board, changing priority. Saw it reorder properly.
- Edited a task on a board in a field of other tasks at the same top-level priority. Saw it refresh without reordering.
Reviewers: chad
Reviewed By: chad
Subscribers: epriestley
Maniphest Tasks: T5476
Differential Revision: https://secure.phabricator.com/D9832
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 T5532. Allow documents to have a preamble in the header which can be used to explain who should sign a document and why.
Particularly, I plan to use this to navigate the corporate vs individual stuff more sensibly.
Test Plan: {F174228}
Reviewers: chad
Reviewed By: chad
Subscribers: epriestley
Maniphest Tasks: T5532
Differential Revision: https://secure.phabricator.com/D9819
Summary: Various fixes as suggested by JSHint.
Test Plan: Eye-balled it.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D9783
Summary:
Ref T3116. Add a Herald action "Require legal signatures" which requires revision authors to accept legal agreements before their revisions can be accepted.
- Herald will check which documents the author has signed, and trigger a "you have to sign X, Y, Z" for other documents.
- If the author has already signed everything, we don't spam the revision -- basically, this only triggers when signatures are missing.
- The UI will show which documents must be signed and warn that the revision can't be accepted until they're completed.
- Users aren't allowed to "Accept" the revision until documents are cleared.
Fixes T1157. The original install making the request (Hive) no longer uses Phabricator, and this satisfies our requirements.
Test Plan:
- Added a Herald rule.
- Created a revision, saw the rule trigger.
- Viewed as author and non-author, saw field UI (generic for non-author, specific for author), transaction UI, and accept-warning UI.
- Tried to accept revision.
- Signed document, saw UI update. Note that signatures don't currently //push// an update to the revision, but could eventually (like blocking tasks work).
- Accepted revision.
- Created another revision, saw rules not add the document (since it's already signed, this is the "no spam" case).
Reviewers: btrahan, chad
Reviewed By: chad
Subscribers: asherkin, epriestley
Maniphest Tasks: T1157, T3116
Differential Revision: https://secure.phabricator.com/D9771
Summary: Fixes T5500. When an image is embedded with `{Fxx, size=full}`, add "max-width: 100%;" so that large images are scaled down to the size of the container. This seems like a better and more reasonable behavior than having them scroll. You can still lightbox them or right-click -> view if you really want the full image.
Test Plan: Dragged window around with a very large `size=full` image. At large window sizes, the image displayed at 100%. At smaller window sizes, the image was scaled to fit.
Reviewers: chad
Reviewed By: chad
Subscribers: epriestley
Maniphest Tasks: T5500
Differential Revision: https://secure.phabricator.com/D9758
Summary: Fixes T5497. Scope these down a little bit so they don't bleed into `{W...}` embeds and such.
Test Plan:
- Viewed a Legalpad document with headers, monospaced stuff, and lists. Looked the same before/after.
- Viewed a comment with headers, monospace, and lists. Looked the same before/after.
- Viewed a `{W..}` embed, now looks sane.
{F171052}
Reviewers: chad
Reviewed By: chad
Subscribers: epriestley
Maniphest Tasks: T5497
Differential Revision: https://secure.phabricator.com/D9757
Summary: Just a quick pass at making Macro/Pholio panels look not broken. May need longer rethinking to be *good*.
Test Plan: Add a Macro Panel
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D9755
Summary: Doing to start to try to remove all the 'purple' PHUIHeaders around Phabricator and see what's left after.
Test Plan:
View each page
{F171007}
Reviewers: epriestley
Reviewed By: epriestley
Differential Revision: https://secure.phabricator.com/D9750
Summary: Toss the hard-codes and use slim tags.
Test Plan: Scoped out task list.
Reviewers: chad
Reviewed By: chad
Subscribers: epriestley
Differential Revision: https://secure.phabricator.com/D9748
Summary: Shaves a pixel for use in ObjectLists.
Test Plan:
UIExamples.
{F170655}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D9746
Summary: Ref T5482. Instead of editing icons and details seaparetly, use a bunch of Javascript to pop a dialog instead.
Test Plan: {F170528}
Reviewers: chad
Reviewed By: chad
Subscribers: epriestley
Maniphest Tasks: T5482
Differential Revision: https://secure.phabricator.com/D9743
Summary:
Standardizes tag rendering in Maniphest and Maniphest/Diffusion list views.
(This might need some size/spacing tweaks, I tried to make it look reasonable.)
Test Plan: See screenshots.
Reviewers: chad
Reviewed By: chad
Subscribers: epriestley
Differential Revision: https://secure.phabricator.com/D9741
Summary:
Generally reduces friction, standardizes, and simplifies this workflow. Particularly, this removes "address" and "phone", which I think we can wait for user demand for.
For logged-in users, we just always use their primary email.
Test Plan: See screenshots.
Reviewers: chad
Reviewed By: chad
Subscribers: epriestley
Differential Revision: https://secure.phabricator.com/D9735
Summary: Ref T2628. There are a few UIs that need updates, but generally I want to show project icons everywhere that we show project names, to more strongly reinforce the ideas of projects being groups/tags/policies/etc.
Test Plan: See screenshot.
Reviewers: chad
Reviewed By: chad
Subscribers: epriestley
Maniphest Tasks: T2628
Differential Revision: https://secure.phabricator.com/D9709
Summary: Provides a base set of shaded object tags for use in Phabricator.
Test Plan:
Lots of Photoshop and Chrome.
{F170252, size=full}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D9737
Summary: See D9719.
Test Plan:
- Used hide/show columns.
- Used "add column".
- Filtered board.
{F170133}
Reviewers: chad
Reviewed By: chad
Subscribers: epriestley
Differential Revision: https://secure.phabricator.com/D9726
Summary: Fixes T5451.
Test Plan:
- Added a custom date field, observed icon render underneath typeahead results.
- Couldn't think of any reasons that typeaheads should be under any of the elements with index 9, 10, 11, or 12.
Reviewers: chad
Reviewed By: chad
Subscribers: epriestley
Maniphest Tasks: T5451
Differential Revision: https://secure.phabricator.com/D9718
Summary: Adds a bit of space
Test Plan: View 2 or more inline comments on same transaction.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D9712
Summary: Ref T5365. Surface disconnects in the UI.
Test Plan:
- Connected, then killed the server.
- Saw disconnected event and appropriate update in the UI.
{F169605}
Reviewers: joshuaspence
Reviewed By: joshuaspence
Subscribers: epriestley
Maniphest Tasks: T5365
Differential Revision: https://secure.phabricator.com/D9706
Summary: Moves PhabricatorActionHeaderView to PHUIActionHeaderView, adds Red, Green, and Violet colors and extend ObjectBox to take colors and action headers.
Test Plan:
Tested new Welcome layout as well as UIExamples, Workboards, and Hovercards
{F169669}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D9707
Summary:
Fixes T5373. Ref T5281. Several changes:
- The `marshallExceptions` thing is useful if JS throws an exception when invoked from Flash, so set it. The resulting exceptions are a little odd (not escaped correctly, e.g.) but way better than nothing.
- Put connection status in the notification menu.
- When the connection fails, try to provide contextual help where we can.
Test Plan: {F169493}
Reviewers: chad, joshuaspence
Reviewed By: joshuaspence
Subscribers: epriestley
Maniphest Tasks: T5281, T5373
Differential Revision: https://secure.phabricator.com/D9700
Summary:
Ref T5373. The control flow between `aphlict-listener` and `JX.Aphlict` is pretty weird right now, where the listener (which is the highest-level component) has intimate knowledge of how to put the SWF on the page.
Instead:
- Make `JX.Aphlict` a real singleton.
- Instantiate it sooner.
- Have it handle the flash setup handshake.
Test Plan: Loaded page in debug mode, saw normal flow take place.
Reviewers: joshuaspence
Reviewed By: joshuaspence
Subscribers: epriestley
Maniphest Tasks: T5373
Differential Revision: https://secure.phabricator.com/D9699
Summary:
Ref T5373. This seems to work pretty much correctly.
Also stop popping bubbles and just use the log, since users find the bubbles confusing/not useful and they're not great for developers either.
Future diffs will expose more user-facing stuff.
Test Plan: Added `throw` to AphlictClient.as, got a log in the parent window.
Reviewers: joshuaspence
Reviewed By: joshuaspence
Subscribers: epriestley
Maniphest Tasks: T5373
Differential Revision: https://secure.phabricator.com/D9698
Summary: I don't believe that this HTML serves a purpose any longer. In addition, loading https://secure.phabricator.com/rsrc/swf/test.html doesn't seem particularly useful.
Test Plan: N/A
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D9691
Summary: As advised by JSHint.
Test Plan: I'm not really sure how to comprehensively test this. It looks okay to me.
Reviewers: chad, epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D9677
Summary: There are a bunch of unused variables in JavaScript files. These were identified with JSHint.
Test Plan: It's pretty hard to test this thoroughly... on inspection, it seems that everything //should// be okay (unless we are doing weird things with the JavaScript).
Reviewers: chad, #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D9676
Summary:
Ref T5317. General idea here is that Dashboards are replacing the home page soon. We think they'll do a good job for normal users, but they aren't very good for new administrators who have just completed an install. In this case, any dashboard we put there by default will be empty and not very useful or helpful. It's also technically a bit messy to build objects by default.
Instead, give new administrators a "Quest Tracker" UI to help them get through things. When they're done with setup steps, they build a dashboard and install it to replace the home page. They can add install-specific welcome messages during this process, so hopefully this will also ease onboarding for non-administrator users.
For now, you have to go to Config > Welcome Screen explicitly to see this UI. We can tweak/test it for a bit before replacing the home page.
Test Plan: {F169226}
Reviewers: btrahan, chad
Reviewed By: chad
Subscribers: epriestley
Maniphest Tasks: T5317
Differential Revision: https://secure.phabricator.com/D9660
Summary: Some styles were getting clobbered, tightened things up
Test Plan: Test mobile, desktop and tablet breakpoints
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D9669
Summary: Adds a top border only if a comment is preceding.
Test Plan: test inlines with and without commentsy
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D9666
Summary: Replaces Embed hint with where the heck you are hint.
Test Plan: Tested current and previous mock images.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Maniphest Tasks: T5384
Differential Revision: https://secure.phabricator.com/D9658
Summary:
Fixes T5424.
- One concrete issue: drafts were not being cleared properly because `__draft__` was not set on submission. This (mostly) fixes phantom drafts.
- This ajax comment magic feels weird and floaty and generally has problems. For example, if you add subscribers or inlines, all the stuff on the page which represents those won't update automatically. Instead, just reload. Maybe we'll ajax this stuff some day, but it feels like a net negative for now.
- Also remove it from other applications where it's currently used.
- Fix an issue with inline previews.
Test Plan: Made some comments on a mock, everything worked normally like I expected it to.
Reviewers: chad
Reviewed By: chad
Subscribers: epriestley
Maniphest Tasks: T5424
Differential Revision: https://secure.phabricator.com/D9649
Summary: Fixes T5437. This actual behavior is debateable but this is the one that seems simplest and most sensible.
Test Plan: Copied some cells from Numbers, pasted them into a remarkup box in Chrome, got just the text.
Reviewers: btrahan, chad
Reviewed By: chad
Subscribers: epriestley
Maniphest Tasks: T5437
Differential Revision: https://secure.phabricator.com/D9647
Summary: Ref T5179. Ref T4045. Ref T832. We can now write non-utf8 hunks into the database, so try to do more reasonable things with them in the UI.
Test Plan: (See screenshots...)
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T832, T4045, T5179
Differential Revision: https://secure.phabricator.com/D9294
Summary:
Minor things
- Fades out comment icon on hover
- Adds hover to inline comment images
- moves mask position to just the image, and not the transparent border
Test Plan: Tested all of these items on various mocks
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D9631
Summary: Adds a PHUI class for display images on a center point, with or without a mask.
Test Plan:
I am bad a math, so like, check that for me please. I tested using Photoshop. Class may need tweaked depending how we store the inline-comment coords.
{F167829}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D9614
Summary: n/a
Test Plan: Added title and description to 1 of 2 mocks, toggled left and right, saw correct CSS.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D9580
Summary:
Ref T4986. Instead of requiring you to know engine class names and copy/paste URLs, provide select dropdowns that use SCARY JAVASCRIPT to do magical things.
I think this is mostly reasonable, the only issue is that it's hard to create a panel out of a completely ad-hoc query (you'd have to save it, then create a panel out of the saved query, then remove the saved query). Once we develop T5307 we can do a better job of this.
Test Plan: See screenshots.
Reviewers: chad
Reviewed By: chad
Subscribers: epriestley
Maniphest Tasks: T4986
Differential Revision: https://secure.phabricator.com/D9572
Summary:
- When the button is clicked, actually download the file or image.
- Add aural hints for the icon-only buttons.
- Use a "photo" icon for "view raw image", so the "arrows pointing outward" icon can be used for "fullscreen" some day.
Test Plan: Clicked link, got a download.
Reviewers: chad
Reviewed By: chad
Subscribers: epriestley
Differential Revision: https://secure.phabricator.com/D9574
Summary: You were right
Test Plan:
mmm, blue
{F167137}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D9567
Summary:
Ref T2644. This adjusts thumb sizing so the "X" button is visible, and hides the uploader on devices for now.
The thumb stuff I'm sort of hacking (we'll cut off a little bit of wide thumbs on the iPhone), but it looks fine, is usable, and works a little better in landscape mode and at tablet sizes.
Test Plan: {F167022}
Reviewers: chad
Reviewed By: chad
Subscribers: epriestley
Maniphest Tasks: T2644
Differential Revision: https://secure.phabricator.com/D9562
Summary: Fixes T5283.
Test Plan: See screenshots.
Reviewers: chad
Reviewed By: chad
Subscribers: epriestley
Maniphest Tasks: T5283
Differential Revision: https://secure.phabricator.com/D9549
Summary:
Ref T5359. When users upload non-image file types (PDFs, text files, whatever), Pholio currently chokes in a few places. Make most of these behaviors more reasonable:
- Provide thumbs in the required sizes.
- Predict the thumb size of these files correctly.
- Disable inline comments.
- Make "View Fullsize" and "Download" into buttons. These mostly-work. Download should probaly really download, but CSRF on forms is a bit of a pain right now.
Test Plan: See screenshots.
Reviewers: chad
Reviewed By: chad
Subscribers: epriestley
Maniphest Tasks: T5359
Differential Revision: https://secure.phabricator.com/D9548
Summary: This is a little rough visually but the actual number works fine.
Test Plan: {F166844}
Reviewers: chad
Reviewed By: chad
Subscribers: epriestley
Differential Revision: https://secure.phabricator.com/D9547
Summary: Gets rid of all the dark css.
Test Plan:
Do it live.
{F166665}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D9545
Summary:
This could probably use some refinement (and, like, explanatory text, and stronger cues about what rows and columns mean) but feels fairly good to me, at least on test data.
I didn't do any scrolling for now since we have to do full height on mobile anyway I think. I did swap it so the newer ones are on top.
Left/right navigate you among current images only, but you can click any thumb to review history.
Removed history view since it's no longer useful.
Some things that would probably help:
- Some kind of header explaining what this is ("Mock History" or something).
- Stronger visual cue that columns are related by being the same image.
- Clearer cues about obsolete/deleted images (e.g., on the stage itself?)
- Maybe general tweaks.
- Maybe a placeholder (like a grey "X") for images which have been deleted.
(I'm planning to add comment counts too, which I think will be pretty useful, but that felt good to put in another diff.)
Test Plan: See screenshots.
Reviewers: chad
Reviewed By: chad
Subscribers: epriestley
Differential Revision: https://secure.phabricator.com/D9543
Summary:
Currently, we limit the image size to make sure that the stage has a constant height and the entire image always fits on screen.
In practice, these don't actually seem like desirable qualities. Instead, focus on giving as many pixels as possible to the image.
Test Plan: See screenshots.
Reviewers: chad
Reviewed By: chad
Subscribers: epriestley
Differential Revision: https://secure.phabricator.com/D9539
Summary:
- Moves the right-hand gutter under the image.
- Moves size information to the upper right.
- This is transitional, on the way toward something more like the mocks in D9534.
Test Plan: See screenshot.
Reviewers: chad
Reviewed By: chad
Subscribers: epriestley
Differential Revision: https://secure.phabricator.com/D9538
Summary: This greatly simplifies inline comments while retaining their functionality. This is probably not where we want to end up, but will let us figure out what we're doing with the stage without worrying about inlines.
Test Plan: See screenshots.
Reviewers: chad
Reviewed By: chad
Subscribers: epriestley
Differential Revision: https://secure.phabricator.com/D9537
Summary:
- Hide inline areas on mobile, and when the mouse cursor is not on the stage.
- Show small icon markers instead of areas.
- Yellow icons show draft comments; pink icons show final comments.
Test Plan: {F166544}
Reviewers: chad
Reviewed By: chad
Subscribers: epriestley
Differential Revision: https://secure.phabricator.com/D9536
Summary: Uses common CSS spacing, tweaks some colors, increases viewport size to very limits.
Test Plan:
Test large and small images, various breakpoints. Able to more easily review mocks.
{F166500}
{F166501}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D9534
Summary: Changes the old dark embed to match the PinboardView. Retains ability to target individual files. Removes "carousel" of files (not super useful?)
Test Plan:
Tested embedding Mocks, with and without targeting specific files. Tested Pholio Pinboard, Macro Pinboard.
{F166451}
{F166452}
{F166453}
{F166454}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D9531
