1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-12-23 05:50:55 +01:00
Commit graph

3131 commits

Author SHA1 Message Date
epriestley
415de4ce37 Repaint filetree more consistently for mobile/device views
Summary:
Ref T13189. See <https://discourse.phabricator-community.org/t/diffusion-differential-mobile-layout-broken-when-enabling-file-tree/1751>.

We currently call a nonexistent `resetdrag()` which does nothing. Some sequences of interactions can result in a blank left column in mobile/device widths.

Repaint the filetree away more consistently on device change.

Test Plan: Viewed a revision, toggled filetree off + on, resized to narrow width. Before: bad left margin, JS console error. After: proper repaint at device breakpoint, no JS console error.

Reviewers: amckinley

Maniphest Tasks: T13189

Differential Revision: https://secure.phabricator.com/D19611
2018-08-27 10:32:47 -07:00
epriestley
8a6d767843 Fix a minor text alignment issue for static text comment actions like "Accept Revision"
Summary:
Ref T13187. See PHI836. The "action" comment actions in Differential (Accept, Reject, etc) render a single line of descriptive text. This is currently slightly misaligned.

Give it similar sizing information to the label element to the left, so it lines up properly.

Test Plan:
Note that "Request Review" and "This revision will be..." are now aligned:

{F5828077}

Reviewers: amckinley

Maniphest Tasks: T13187

Differential Revision: https://secure.phabricator.com/D19600
2018-08-24 10:12:06 -07:00
epriestley
75a5dd8d8c Add more accessibility labels for screen readers
Summary:
Depends on D19594. See PHI823. Ref T13164.

  - Add a label for the "X" button in comment areas, like "Remove Action: Change Subscribers".
  - Add a label for the floating header display options menu in Differential.
  - Add `role="button"` to `PHUIButtonView` objects that we render with an `<a ...>` tag.

Test Plan:
Viewed a revision with `?__aural__=true`:

  - Saw "Remove Action: ..." label.
  - Saw "Display Options" label.
  - Used inspector to verify that some `<a class="button" ...>` now have `<a class="button" role="button" ...>`. This isn't exhaustive, but at least improves things. A specific example is the "edit", "reply", etc., actions on inline comments.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13164

Differential Revision: https://secure.phabricator.com/D19595
2018-08-17 13:31:51 -07:00
epriestley
9a15129b40 Remove 750ms timeout on owners path validation
Summary:
Ref T13164. See PHI748. Path validation has a 750ms timeout which blames to rP5038ab850c, in 2011.

Production path validation is sometimes taking more than 750ms, particularly on the initial page load where we may validate many paths simultaneously.

I have no idea why we have this timeout, and it isn't consistent with how we perform other AJAX requests. Just remove it.

Test Plan:
  - Reproduced issue in production, saw all validation calls failing at 750ms. Actual underlying calls succeed, they just take more than 750ms to resolve.
  - Loaded path validator locally, got green checkmark.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13164

Differential Revision: https://secure.phabricator.com/D19575
2018-08-13 13:52:26 -07:00
epriestley
e5906f4e12 In Differential standalone views, disable some keyboard shortcuts which don't work
Summary:
Ref T13164. See PHI693. In Differential, you can {nav View Options > View Standalone} to get a standalone view of a single changeset. You can also arrive here via the big changeset list for revisions affecting a huge number of files.

We currently suggest that all the keyboard shortcuts work, but some do not. In particular, the "Next File" and "Previous File" keyboard shortcuts (and some similar shortcuts) do not work. In the main view, the next/previous files are on the same page. In the standalone view, we'd need to actually change the URI.

Ideally, we should do this (and, e.g., put prev/next links on the page). As a first step toward that, hide the nonfunctional shortcuts to stop users from being misled.

Test Plan:
  - Viewed a revision in normal and standalone views.
  - No changes in normal view, and all keys still work ("N", "P", etc).
  - In standalone view, "?" no longer shows nonfunctional key commands.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13164

Differential Revision: https://secure.phabricator.com/D19571
2018-08-13 08:59:05 -07:00
Austin McKinley
a6951a0a5a Add migration to encourage rebuilding repository identities
Summary: Ref T12164. Defines a new manual activity that suggests rebuilding repository identities before Phabricator begins to rely on them.

Test Plan:
- Ran migration, observed expected setup issue: {F5788217}
- Ran `bin/config done identities` and observed setup issue get marked as done.
- Ran `/bin/storage upgrade --apply phabricator:20170912.ferret.01.activity.php` to make sure I didn't break the reindex migration; observed reindex setup issue appear as expected.
- Ran `./bin/config done reindex` and observed reindex issue cleared as expected.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin, PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T12164

Differential Revision: https://secure.phabricator.com/D19497
2018-08-10 13:47:03 -07:00
epriestley
8d8086fccf Add Spaces support to Phriction
Summary:
Ref T13164. See PHI774. Fixes T12435.

Since Phriction is hierarchical, there isn't a super strong motivation to support Spaces: you can generally set policies on a small number of documents to get the desired effective policy behavior.

However, it still improves consistency and there's no reason //not// to support Spaces. In the case where you have some moderately weird/complex policy on one or more Spaces, using Spaces to define the policy behavior can make things a bit simpler and easier to understand.

This probably doesn't actually fix whatever the root problem in T12435 was (complicated, non-hierarchical access policies?). See also a bunch of discussion in T12442. So we might end up going beyond this to address other use cases, but I think this is reasonable regardless.

Test Plan: Created and edited Phriction documents and shifted them between Spaces. Searched by Space, etc.

Reviewers: amckinley

Reviewed By: amckinley

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13164, T12435

Differential Revision: https://secure.phabricator.com/D19553
2018-07-31 10:24:28 -07:00
epriestley
8374201620 Add a more specific CSS rule to make Spaces headers in projects colored red
Summary:
Depends on D19551. Ref T13164. Projects use a special kind of header setup that has a more specific CSS rule to make content black. Add an even more specific rule to make it red.

(This could probably be disentangled a bit and isn't necessarily the cleanest fix, but I poked at it for a few minutes and didn't come up with anything cleaner.)

Test Plan: Viewed projects in spaces, saw the space names colored red properly.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13164

Differential Revision: https://secure.phabricator.com/D19552
2018-07-31 10:23:35 -07:00
epriestley
13cac5c362 Add Spaces to Projects
Summary:
See PHI774. Ref T13164. There is no reason projects //don't// support Spaces, just a vague concern that it's not hugely useful and might be a bit confusing.

However, it's at least somewhat useful (to improve consistency and reduce special casing) and doesn't necessarily seem more confusing than Projects are anyway. Support is trivial from a technical point of view, so just hook it up.

Test Plan: Created new projects, shifted projects between spaces. The support is all pretty much automatic.

Reviewers: amckinley

Reviewed By: amckinley

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13164

Differential Revision: https://secure.phabricator.com/D19549
2018-07-31 10:15:41 -07:00
epriestley
4e84d4d458 Allow the haunted comment panel ("Z") to take up more vertical room
Summary:
Ref T13151. See PHI685. When you haunt the panel, we only let it take up part of the screen. Let it take up slightly more of the screen so that it's more likely to fit completely on-screen without needing to scroll.

The behavior when it does scroll is fine (you get a scrollbar if your OS/browser is set up to show them) so this is a bit trivial/silly, but seems fine and doesn't have a big JS maintenance cost or anything.

Test Plan: Pressed "Z", resized my window to a weird tiny useless size, got slightly better (I guess) behavior.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13151

Differential Revision: https://secure.phabricator.com/D19480
2018-06-07 13:19:35 -07:00
epriestley
7729c51cc4 Fix an issue where scrolling down, then up, then down fails to show changeset header in Differential
Summary: Ref T13151. See PHI616. There's a bug where the current banner changeset isn't cleared correctly when we hide the banner.

Test Plan:
  - View revision with several changesets.
  - Scroll down slowly through first changeset until banner appears.
  - Scroll up until banner disappears.
  - Scroll back down.
  - Before: banner fails to reappear (code still thinks it's visible and we don't want to update it).
  - After: banner reappears correctly.

Reviewers: amckinley, jmeador

Reviewed By: jmeador

Maniphest Tasks: T13151

Differential Revision: https://secure.phabricator.com/D19474
2018-06-07 12:02:18 -07:00
epriestley
a894c99935 Add "max-width: 100%;" to stop large images from overflowing the new rendering engine UI
Summary:
Fixes T13148. Ref T13105. The new document rendering engine for images let them overflow the UI bounds.

Add `max-width: 100%;` to keep them contained.

Test Plan:
  - Viewed a very wide image in Safari, Firefox and Chrome. Saw sensible rendering.
  - Also viewed a normal image, saw normal behavior.

Reviewers: amckinley, avivey

Reviewed By: avivey

Maniphest Tasks: T13148, T13105

Differential Revision: https://secure.phabricator.com/D19457
2018-06-01 14:53:10 -07:00
epriestley
31ee49b14d Fix Javascript busy loop when trying to delete tokens from an empty tokenizer
Summary:
Fixes T13147. In D19437, I changed this logic to support deleting the `""` (empty string) token, but `[].pop()` returns `undefined`, not `null`, if the list is empty and I didn't think to try deleting an empty input.

Fix the logic so we don't end up in a loop if the input is empty.

Test Plan:
  - In any browser, deleted all tokens in a tokenizer; then pressed delete again.
  - Before: tab hangs in an infinte loop.
  - After: smooth sailing.

Reviewers: amckinley, avivey

Reviewed By: avivey

Maniphest Tasks: T13147

Differential Revision: https://secure.phabricator.com/D19456
2018-06-01 14:51:06 -07:00
Austin McKinley
fe5fde5910 Assign RepositoryIdentity objects to commits
Summary: Depends on D19429. Depends on D19423. Ref T12164. This creates new columns `authorIdentityPHID` and `committerIdentityPHID` on commit objects and starts populating them. Also adds the ability to explicitly set an Identity's assignee to "unassigned()" to null out an incorrect auto-assign. Adds more search functionality to identities. Also creates a daemon task for handling users adding new email address and attempts to associate unclaimed identities.

Test Plan: Imported some repos, watched new columns get populated. Added a new email address for a previous commit, saw daemon job run and assign the identity to the new user. Searched for identities in various and sundry ways.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin, PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T12164

Differential Revision: https://secure.phabricator.com/D19443
2018-05-31 07:28:23 -07:00
Austin McKinley
f191a66490 Add controllers/search/edit engine functionality to RepositoryIdentity
Summary: Depends on D19423. Ref T12164. Adds controllers capable of listing and editing `PhabricatorRepositoryIdentity` objects. Starts creating those objects when commits are parsed.

Test Plan: Reparsed some revisions, observed objects getting created in the database. Altered some `Identity` objects using the controllers and observed effects in the database. No attempts made to validate behavior under "challenging" author/committer strings.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin, PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T12164

Differential Revision: https://secure.phabricator.com/D19429
2018-05-31 07:03:25 -07:00
Austin McKinley
cd84e53c44 Begin building out RepositoryIdentity indirection layer
Summary: Ref T12164. Start building initial objects for managing `RepositoryIdentity` objects. This won't land until much more of the infrastructure is in place.

Test Plan: Ran `bin/storage upgrade` and observed expected table.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin, PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T12164

Differential Revision: https://secure.phabricator.com/D19423
2018-05-31 07:01:16 -07:00
epriestley
d280b24239 Fix "arc paste" to stop creating pastes with an empty string ("") as the "language"
Summary:
See PHI652. When you `echo x | arc paste` today, you end up with a Paste object that has the empty string as its "language".

This is normally not valid. Pastes where the language should be autodetected should have the value `null`, not the empty string.

This behavior likely changed when `paste.create` got rewritten in terms of `paste.edit`. Adjust the implementation so it only adds the LANGUAGE transaction if there's an actual language.

Also, fix an issue where you can't use the "delete" key to delete tokens with the empty string as their value.

Test Plan:
  - Created a paste with `echo x | arc paste`, got a paste in autodetect mode instead of with a bogus language value.
  - Created a paste with `echo x | arc paste --lang rainbow`, got a rainbow paste.
  - Deleted an empty string token with the keyboard.
  - Deleted normal tokens with the keyboard.
  - Edited subscribers/etc normally with the keyboard and mouse to make sure I didn't ruin anything.

Reviewers: amckinley

Reviewed By: amckinley

Differential Revision: https://secure.phabricator.com/D19437
2018-05-09 13:22:58 -07:00
epriestley
26c0db8dd7 Allow navigation breadcrumbs to be marked as "always visible" so they show up on phones
Summary:
See PHI624. Some of the mobile navigation and breadcrumbs in support pacts aren't as good as they could be.

In particular, we generally collapse crumbs on mobile to just the first and last crumbs. The first crumb is the application; the last is the current page.

On `/PHIxxx` pages, the first crumb isn't very useful since the Support landing page is two levels up: you usually want to go back to the pact, not all the way back to the Support landing page.

We also don't need the space since the last crumb (`PHIxxx`) is always small.

Allow Support and other similar applications to tailor the crumb behavior more narrowly if they end up in situations like this.

Test Plan:
  - With an additional change to instances (see next diff), viewed a support issue page (`/PHI123`) on mobile and desktop.
  - Saw a link directly back to the pact on both mobile and desktop.

Reviewers: amckinley

Reviewed By: amckinley

Differential Revision: https://secure.phabricator.com/D19438
2018-05-09 13:21:47 -07:00
epriestley
4a98e0ff65 Allow Owners packages to be configured to ignore generated paths in Differential
Summary:
Depends on D19427. Ref T13130. See PHI251. Support configuring owners packages so they ignore generated paths.

This is still a little rough. A couple limitations:

  - It's hard to figure out how to use this control if you don't know what it's for, but we don't currently have a "CheckboxesEditField". I may add that soon.
  - The attribute ignore list doesn't apply to Diffusion, only Differential, which isn't obvious. I'll either try to make it work in Diffusion or note this somewhere.
  - No documentation yet (which could mitigate the other two issues a bit).

But the actual behavior seems to work fine.

Test Plan:
  - Set a package to ignore paths with the "generated" attribute. Saw the package stop matching generated paths in Differential.
  - Removed the attribute from the ignore list.
  - Tried to set invalid attributes, got sensible errors.
  - Queried a package with Conduit, got the ignored attribute list.

Reviewers: amckinley

Reviewed By: amckinley

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13130

Differential Revision: https://secure.phabricator.com/D19428
2018-05-05 08:47:29 -07:00
epriestley
dc510354c3 Remove explicit "mailKey" from Owners packages
Summary:
Depends on D19426. Ref T13130. Ref T13065. While I'm making changes to Owners for "Ignore generated paths", clean up the "mailKey" column.

We recently (D19399) added code to automatically generate and manage mail keys so we don't need a ton of `mailKey` properties in the future. Migrate existing mail keys and blow away the explicit column on packages.

Test Plan: Ran migration, manually looked at the database and saw sensible data. Edited a package to send some mail, which looked good.

Reviewers: amckinley

Reviewed By: amckinley

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13130, T13065

Differential Revision: https://secure.phabricator.com/D19427
2018-05-05 08:47:08 -07:00
epriestley
afc3099ee7 Add a view option to disable blame in Diffusion and fix some view transition bugs
Summary:
See PHI604. Ref T13130. Ref T13105. There's currently no way to turn blame off in Diffusion. Add a "Hide Blame" option to the "View Options" dropdown so it can be toggled off.

Also fix a couple of bugs around this: for example, if you loaded a Jupyter notebook and then switched to "Source" view, blame would incorrectly fail to activate because the original rendering of the "stage" used an asynchronous engine so `willRenderRef()` wasn't called to populate blame.

Test Plan:
  - Viewed a source file, toggled blame off/on, reloaded page to see state stick in URL.
  - Viewed a Jupyter notebook, toggled to "Source" view, saw blame.
  - Viewed stuff in Files (no blame UI options).
  - Tried to do some invalid stuff like toggle blame on a non-blame engine (options disable properly).

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13130, T13105

Differential Revision: https://secure.phabricator.com/D19414
2018-04-30 15:32:23 -07:00
epriestley
28517110c6 Fix an issue in the new Harbormaster build log view where clicking the "^" icon doesn't work right
Summary:
Ref T13130. See PHI617.

The new build log UI has tags like `<a href="...">Show More Above <span icon>^</span></a>`. If you click the little "^" icon, the event target is the `<span />` instead of the `<a />` so we expand on the wrong node.

Instead, select the `<a />` by sigil explicitly.

Test Plan: Viewed new log UI in Harbormaster, clicked "^" icon and text, got the same (correct) behavior on both.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13130

Differential Revision: https://secure.phabricator.com/D19410
2018-04-27 11:51:59 -07:00
epriestley
1b24b486f5 Manage object mailKeys automatically in Mail instead of storing them on objects
Summary:
Ref T13065. `mailKey`s are a private secret for each object. In some mail configurations, they help us ensure that inbound mail is authentic: when we send you mail, the "Reply-To" is "T123+456+abcdef".

  - The `T123` is the object you're actually replying to.
  - The `456` is your user ID.
  - The `abcdef` is a hash of your user account with the `mailKey`.

Knowing this hash effectively proves that Phabricator has sent you mail about the object before, i.e. that you legitimately control the account you're sending from. Without this, anyone could send mail to any object "From" someone else, and have comments post under their username.

To generate this hash, we need a stable secret per object. (We can't use properties like the PHID because the secret has to be legitimately secret.)

Today, we store these in `mailKey` properties on the actual objects, and manually generate them. This results in tons and tons and tons of copies of this same ~10 lines of code.

Instead, just store them in the Mail application and generate them on demand. This change also anticipates possibly adding flags like "must encrypt" and "original subject", which are other "durable metadata about mail transmission" properties we may have use cases for eventually.

Test Plan:
  - See next change for additional testing and context.
  - Sent mail about Herald rules (next change); saw mail keys generate cleanly.
  - Destroyed a Herald rule with a mail key, saw the mail properties get nuked.
  - Grepped for `getMailKey()` and converted all callsites I could which aren't the copy/pasted boilerplate present in 50 places.
  - Used `bin/mail receive-test --to T123` to test normal mail receipt of older-style objects and make sure that wasn't broken.

Reviewers: amckinley

Reviewed By: amckinley

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13065

Differential Revision: https://secure.phabricator.com/D19399
2018-04-25 06:46:58 -07:00
epriestley
33da9f833f Fix odd line number line wrapping on embedded pastes ({Pxxx})
Summary: Ref T13126. After SourceView changes, embedded pastes with the `{Pxxx}` syntax are line-wrapping line numbers in Safari, at least. Put a stop to this.

Test Plan: Viewed a `{Pxxx}` with more than 10 lines. Before: weird line wrapping; after: nice consistent display.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13126

Differential Revision: https://secure.phabricator.com/D19393
2018-04-20 14:20:20 -07:00
Austin McKinley
4dc8e2de56 Add unique constraint to AlmanacInterfaces
Summary: See discussion in D19379. The 4-tuple of (device, network, address, port) should be unique.

Test Plan: Created lots of duplicate interfaces, bound those interfaces to various services, observed migration script clean things up correctly.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin, PHID-OPKG-gm6ozazyms6q6i22gyam

Differential Revision: https://secure.phabricator.com/D19388
2018-04-19 19:16:50 -07:00
epriestley
19403fdb8e Improve color use in "[+++- ]" element for colorblind users
Summary:
Ref T13127. Users with red/green colorblindness may have difficulty using this element in its current incarnation.

We could give it different behavior if the "Accessibility" option is set for red/green colorblind users, but try a one-size-fits-all approach since the red/green aren't wholly clear anwyay.

Test Plan: {F5530050}

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13127

Differential Revision: https://secure.phabricator.com/D19385
2018-04-19 17:24:44 -07:00
epriestley
70d67a3908 Fix the most significant "phantom notification" badness
Summary:
Ref T13124. Ref T13131. Fixes T8953. See PHI512.

When you receieve a notification about an object and then someone hides that object from you (or deletes it), you get a phantom notification which is very difficult to clear.

For now, test that notifications are visible when you open the menu and clear any that are not.

This could be a little more elegant than it is, but the current behavior is very clearly broken. This unbreaks it, at least.

Test Plan:
  - As Alice, configured task stuff to notify me (instead of sending email).
  - As Bailey, added Alice as a subscriber to a task, then commented on it.
  - As Alice, loaded home and saw a notification count. Didn't click it yet.
  - As Bailey, set the task to private.
  - As Alice, clicked the notification bell menu icon.
    - Before change: no unread notifications, bell menu is semi-stuck in a phantom state which you can't clear.
    - After change: bad notifications automatically cleared.

{F5530005}

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13131, T13124, T8953

Differential Revision: https://secure.phabricator.com/D19384
2018-04-19 17:24:19 -07:00
Austin McKinley
e81b2173ad Add edge tables for Phlux
Summary: Fixes T13129. This at least makes the existing UI work again before we banish Phlux to the shadow realm.

Test Plan: Edited the visibility for a Phlux variable, didn't get an error. Nothing showed up in the edge tables when I made those changes, but at least it doesn't error out anymore.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin, PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13129

Differential Revision: https://secure.phabricator.com/D19387
2018-04-19 15:49:08 -07:00
Austin McKinley
0a83f253ed Add unique constraint for Almanac network names
Summary:
The name of networks should be unique.

Also adds support for exact-name queries for AlamanacNetworks.

Test Plan: Applied migration with existing duplicates, saw networks renamed, attempted to add duplicates, got a nice error message.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin, PHID-OPKG-gm6ozazyms6q6i22gyam

Differential Revision: https://secure.phabricator.com/D19379
2018-04-19 13:41:15 -07:00
epriestley
665529ab60 Restore coverage reporting to Diffusion browse UI
Summary:
Depends on D19377. Ref T13125. Ref T13124. Ref T13105. Coverage reporting in Diffusion didn't initially survive the transition to Document Engine; restore it.

This adds some tentative/theoretical support for multiple columns of coverage, but no way to actually produce them in the UI. For now, the labels, codes, and colors are hard coded.

Test Plan:
Added coverage with `diffusion.updatecoverage`, saw coverage in the UI:

{F5525542}

Hovered over coverage, got labels and highlighting.

Double-checked labels for "N" (Not Executable) and "U" (Uncovered). See PHI577.

Faked some multi-column coverage, but you can't currently get this yourself today:

{F5525544}

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13125, T13124, T13105

Differential Revision: https://secure.phabricator.com/D19378
2018-04-17 14:51:47 -07:00
epriestley
21bb0215db Remove obsoleted "diffusion-browse-file" behavior for coverage
Summary: Ref T13105. After moving Diffusion to DocumnentEngine, this no longer has callers. It will become part of the document behavior.

Test Plan: Grepped for calls to the `diffusion-browse-file` behavior, found none.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13105

Differential Revision: https://secure.phabricator.com/D19377
2018-04-17 14:51:12 -07:00
epriestley
37a03402bc When following a link to a particular line ("/example.txt$12"), scroll to that line
Summary:
Depends on D19349. Ref T13105. This was the behavior in Diffusion before with a little hard-coded snippet.

Remove that snippet ("diffusion-jump-to") and add a more general-purpose snippet to SourceView.

This is a tiny bit hacky still (and probably doesn't work quite right with Quicksand) but gets things working again and works in all of Files, Paste, and Diffusion.

Test Plan: Followed links to particular lines in Paste, Files and Diffusion; got scrolled to the right place.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13105

Differential Revision: https://secure.phabricator.com/D19350
2018-04-11 17:29:22 -07:00
epriestley
5b3a351852 Use pseudoelements, not Zero Width Space, to implement copy/paste behavior in Paste/Diffusion
Summary:
Depends on D19348. Ref T13105. When copying text from Paste or Diffusion, we'd like to copy only source, not line numbers.

We currently accomplish this with zero-width spaces plus a trigger that fires on "copy" in Paste and Diffusion. This is quite gross.

In the new-style Harbormaster logs, we use an approach that seems slightly better: CSS psuedoelements.

This isn't a complete solution (see also PHI504 / T5032) but puts us in a slightly better place.

Use it in Paste/Files/Diffusion too.

This gives us good behavior in all browsers in Files and Paste.

This gives us good behavior in Chrome and Firefox in Diffusion. Safari will copy (but not visually select) blame information in Diffusion. I think we can live with that for now.

Test Plan: Selected and copy/pasted stuff in Diffusion, Files, and Paste. Got good behavior everywhere except Safari + Diffusion.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13105

Differential Revision: https://secure.phabricator.com/D19349
2018-04-11 17:28:46 -07:00
epriestley
c5c53e277a Make line selection in source code views less fragile and more consistent
Summary:
Depends on D19347. Ref T13105. See PHI565. The "highlight lines" behavior is interacting poorly with the new blame element in Diffusion.

Make the behavior a little simpler and hopefully more robust.

Test Plan:
  - Clicked commit/revision links in Diffusion, saw the links get followed instead of the lines highlighted.
  - Highlighted lines in Diffusion, saw just the line/code highlight instead of the whole thing.
  - Highlighted lines in Paste and new-style Harbormaster build logs, saw consistent behavior.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13105

Differential Revision: https://secure.phabricator.com/D19348
2018-04-11 17:27:34 -07:00
epriestley
ac570fd4bc When you make the file tree huge, scroll to the right, and then toggle it, stop it from growing
Summary: Depends on D19346. Ref PHI568. I love Javascript.

Test Plan:
  - Viewed a revision.
  - Dragged file tree view really wide.
  - Scrolled document to the right.
  - Toggled file tree off and on by pressing "f" twice.
    - Before patch: file tree grew wider and wider after it was toggled.
    - After patch: file tree stayed the same size after it was toggled.
  - Dragged to various widths and reloaded to make sure the "sticky across reloads" behavior still works.
  - Scrolled right, dragged the tree a bit, then reloaded and didn't see it flip out.

Reviewers: amckinley

Reviewed By: amckinley

Differential Revision: https://secure.phabricator.com/D19347
2018-04-11 17:25:31 -07:00
epriestley
55619e8964 Restore an explicit white background color to files in Paste
Summary:
Ref T13105. Previously, the "source code" view in Paste rendered on a brown/orange-ish background. I've been using this element in more contexts (Files, Diffusion) and removed the colored background to make text (particularly syntax-highlighted text) easier to read and reduce visual noise with the new blame colors.

In Diffusion the view is in a box with a white background so removing the background left us with white, but in Paste it's just directly on the page so the background was bleeding through. Instead, set it to white explicitly.

Test Plan: Viewed source files in Files, Diffusion and Paste; saw text on a white background.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13105

Differential Revision: https://secure.phabricator.com/D19346
2018-04-11 17:21:33 -07:00
epriestley
d6ef32a7b7 Give the "Filetree" UI element an explicit background color
Summary:
See PHI568. If you make the file tree UI very wide so that the page generates a horizontal scrollbar and then scroll the page, the page content can paint underneath the menu.

The menu already has a z-index to make it render above the content, but doesn't actually have a background. Give it a background.

The "transparent" rule was added in D16346 but I don't see any reason why we actually need it there, so I think this probably won't break anything.

Test Plan: {F5518822}

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13120

Differential Revision: https://secure.phabricator.com/D19344
2018-04-11 10:42:41 -07:00
epriestley
f9c6a69d9c Add skeleton code for Almanac Interfaces to have real transactions
Summary:
Depends on D19322. Ref T13120. Ref T12414.

Currently, `AlmanacDevice` has a bit of a beast of a `TYPE_INTERFACE` transaction that fully creates a complex Interface object. This isn't very flexible or consistent, and Interfaces are complex enough to reasonably have their own object behaviors (for example, they have their own PHIDs).

The complexity of this transaction makes modularizing `AlmanacDevice` transactions tricky. To simplify this, move Interface toward having its own set of normal transactions.

This change just adds some reasonable-looking transactions; it doesn't actually hook them up in the UI or make them reachable. I'll test that they actually work as I swap the UI over.

We may also have some code using the `TYPE_INTERFACE` transaction in Phacility support stuff, so that may need to wait a week to actually phase out.

Test Plan: Ran `bin/storage upgrade` and `arc liberate`. This code isn't reachable yet.

Reviewers: amckinley

Reviewed By: amckinley

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13120, T12414

Differential Revision: https://secure.phabricator.com/D19323
2018-04-11 10:29:26 -07:00
epriestley
4c4a5a7656 Fix the wrapping/padding behavior of Remarkup code block headers more thoroughly?
Summary: Ref T13118. The first fix there fixed Safari, but made Chrome weird. Try this?

Test Plan: Viewed a code block with `name=...` in Safari, Firefox and Chrome and saw consistent display without weird wrappping.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13118

Differential Revision: https://secure.phabricator.com/D19319
2018-04-10 04:37:14 -07:00
epriestley
472bc3d90a Colorize lines in blame under DocumentEngine, to show relative age of changes
Summary:
Depends on D19313. Ref T13105. Fixes T13015. We lost the coloration for ages in the switch to Document Engine.

Restore it, and use a wider range of colors to make the information more clear.

Test Plan: Viewed some blame, saw a nice explosion of bright colors. This is a cornerstone of good design.

Maniphest Tasks: T13105, T13015

Differential Revision: https://secure.phabricator.com/D19314
2018-04-09 06:11:47 -07:00
epriestley
cf75d63b49 When lines 12, 13, 14, etc all blame to the same change, only show it once
Summary:
Depends on D19312. Ref T13105. For readability, render only one link for each contiguous block of changes.

Also make the actual rendering logic a little more defensible.

Test Plan: Viewed some files with blame, saw one render per chunk instead of one per line.

Maniphest Tasks: T13105

Differential Revision: https://secure.phabricator.com/D19313
2018-04-09 06:11:06 -07:00
epriestley
eb80f0a2d9 When you swap between document rendering engines, populate or redraw blame if appropriate
Summary: Depends on D19311. Ref T13105. Currently, blame only renders on the initial request. Instead, redraw blame after swapping views.

Test Plan: Swapped from "Source -> Hexdump -> Source" and "Hexdump -> Source". Saw blame on source in all cases.

Maniphest Tasks: T13105

Differential Revision: https://secure.phabricator.com/D19312
2018-04-09 06:10:41 -07:00
epriestley
11664277b3 Make DocumentEngine source line linking behavior better when blame is shown
Summary: Ref T13105. The line linker behavior currently has trouble identifying the line number when blame is active. Improve this, albeit not the most cleanly.

Test Plan: Selected lines with blame on.

Maniphest Tasks: T13105

Differential Revision: https://secure.phabricator.com/D19310
2018-04-09 06:09:40 -07:00
epriestley
09c6d42b95 Mostly make blame work with DocumentEngine
Summary: Ref T13105. This needs refinement but blame sort of works again, now.

Test Plan: Viewed files in Diffusion and Files; saw blame in Diffusion when viewing in source mode.

Reviewers: mydeveloperday

Reviewed By: mydeveloperday

Maniphest Tasks: T13105

Differential Revision: https://secure.phabricator.com/D19309
2018-04-09 04:48:21 -07:00
epriestley
90a614778c Make repository symbol references work with DocumentEngine
Summary: Ref T13105. Ref T13047. This makes symbol indexes work with DocumentEngine in Files, and restores support in Diffusion.

Test Plan: Command-clicked stuff, got taken to the symbol index with reasonable metadata in Diffusion, Differential and Files.

Reviewers: mydeveloperday

Reviewed By: mydeveloperday

Maniphest Tasks: T13105, T13047

Differential Revision: https://secure.phabricator.com/D19307
2018-04-09 04:47:28 -07:00
epriestley
6dea2ba3b3 Fix DocumentEngine line behaviors in Diffusion
Summary:
Ref T13105. Fixes some issues with line linking and highlighting under DocumentEngine:

  - Adding `$1-3` to the URI didn't work correctly with query parameters.
  - Reading `$1-3` from the URI didn't work correctly because Diffusion parses them slightly abnormally.

Test Plan: Clicked/dragged lines to select them. Observed URI. Reloaded page, got the right selection.

Reviewers: mydeveloperday

Reviewed By: mydeveloperday

Maniphest Tasks: T13105

Differential Revision: https://secure.phabricator.com/D19305
2018-04-09 04:46:47 -07:00
epriestley
fc103f71e9 Fix very odd wrapping / linebreaking for Remarkup code block headers in Safari
Summary: Fixes T13118. Ref T13120. This construction is a little odd; I'm not entirely sure why Safari is doing what it's doing, but this appears to fix it.

Test Plan: Viewed blocks like those in T13118 in Safari. Before the patch, weird last-letter wrapping. After the patch, sensible behavior.

Maniphest Tasks: T13118, T13120

Differential Revision: https://secure.phabricator.com/D19303
2018-04-08 06:15:58 -07:00
epriestley
e70c9f72a4 Show revision sizes using a perplexing, inexplicable symbol code
Summary: Ref T13110. See PHI230. Show revision sizes on a roughly logarithmic scale from 1-7 stars. See D16322 for theorycrafting on this element.

Test Plan: Looked at some revisions, saw plausible-looking size markers.

Maniphest Tasks: T13110

Differential Revision: https://secure.phabricator.com/D19294
2018-04-03 12:49:27 -07:00
epriestley
615d27c8e9 Show an additional "Draft" tag on non-broadcasting revisions in a non-draft state
Summary:
Depends on D19284. Ref T13110. It's now possible to get a revision into a "Abandoned + But, Never Promoted From Draft" state. Show this in the header and provide the draft hint above the comment area.

Also, remove `shouldBroadcast()`. The method `getShouldBroadcast()` now has the same meaning.

Finally, migrate existing drafts to `shouldBroadcast = false` and default `shouldBroadcast` to `true`. If we don't do this, every older revision becomes a non-broadcasting revision because this flag was not explicitly set on revision creation before, only on promotion out of draft.

Test Plan: Ran migration; abandoned draft revisions and ended up in a draft + abandoned state.

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13110

Differential Revision: https://secure.phabricator.com/D19285
2018-04-03 11:09:49 -07:00
epriestley
7189cb7ba8 Support text encoding and syntax highlighting options in document rendering
Summary: Depends on D19273. Ref T13105. Adds "Change Text Encoding..." and "Highlight As..." options when rendering documents, and makes an effort to automatically detect and handle text encoding.

Test Plan:
  - Uploaded a Shift-JIS file, saw it auto-detect as Shift-JIS.
  - Converted files between encodings.
  - Highlighted various things as "Rainbow", etc.

Maniphest Tasks: T13105

Differential Revision: https://secure.phabricator.com/D19274
2018-03-30 11:28:52 -07:00