Summary:
Ref T2009. This is another almost-identical copy of the row scaffolding, which has the same 1up/2up bugs as the 8 other copies of this code.
Turn the "undo" element into an InlineCommentView so we can scaffold it.
Then, scaffold it with the same code as everything else.
Test Plan: Hit "Undo", swapped from 1up to 2up, hit "undo" again, swapped back, tried left/right, everything rendered with proper scaffolding.
Reviewers: btrahan, chad
Reviewed By: chad
Subscribers: epriestley
Maniphest Tasks: T2009
Differential Revision: https://secure.phabricator.com/D12019
Summary: Ref T2009. This has two more copies of the scaffolding.
Test Plan: Created, edited, deleted, replied to inline comments.
Reviewers: btrahan, chad
Reviewed By: chad
Subscribers: epriestley
Maniphest Tasks: T2009
Differential Revision: https://secure.phabricator.com/D12018
Summary:
Ref T1460. Track and store which comments are threaded replies to other comments, vs merely appearing on the same lines.
This doesn't actually write `hasReplies` yet, since that needs to happen when we un-draft comments on submission.
Test Plan: Made inline comments in Differential and Diffusion, including replies. Replies were marked as "Is Reply".
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T1460
Differential Revision: https://secure.phabricator.com/D12017
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
