Summary: Ref T3612. Doesn't render correctly, need help please. Adds a download icon into the renderfilelinkview to allow easier downloads.
Test Plan: Click on link, get download, click on file, get lightbox.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T3612
Differential Revision: https://secure.phabricator.com/D16980
Summary:
Ref T11114. When a user selects "Accept", and then selects "Reject", remove the "Accept". It does not make sense to both accept and reject a revision.
For now, every one of the "actions" conflicts: accept, reject, resign, claim, close, commandeer, etc, etc. I couldn't come up with any combinations that it seems like users are reasonably likely to want to try, and we haven't received combo-action requests in the past that I can recall.
Test Plan:
- Selected "Accept", then selected "Reject". One replaced the other.
- Selected "Accept", then selected "Change Subscribers". Both co-existed happily.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T11114
Differential Revision: https://secure.phabricator.com/D17132
Summary:
Ref T11114. Recent changes broke the links to jump to inline comments from the previews because they get hooked up with JS.
Restore the linking behavior.
Test Plan: Clicked "View" on an inline comment preview, jumped to that comment.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T11114
Differential Revision: https://secure.phabricator.com/D17131
Summary: Ref T11114. This comments nearly working on EditEngine. Only significant issue I caught is that the "View" link doesn't render properly because it depends on JS which is tricky to hook up. I'll clean that up in a future diff.
Test Plan: {F2279201}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T11114
Differential Revision: https://secure.phabricator.com/D17116
Summary: Ref T11114. This begins restoring comment actions to Differential, but on top of EditEngine.
Test Plan: {F2263148}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T11114
Differential Revision: https://secure.phabricator.com/D17107
Summary:
Fixes T12049. This expands "Haunted" comment panels to EditEngine, and by extension to all EditEngine applications.
Eventual goal is to remove custom commenting code in Differential and replace it with EditEngine code.
Changes from current "haunt" mode:
- This only has one mode ("pinned"), not two ("pinned", "pinned with preview"). There's an inline preview and scroll behavior is a little better.
- Now has a UI action button.
Slightly tricky stuff:
- This interacts with "Fullscreen" mode since it doesn't make sense to pin a full-screen comment area.
- This should only be available for comments, not for remarkup fields like "Description" in "Edit Task".
Test Plan:
- Pinned/unpinned in Maniphest.
- Pinned/fullscreened/unfullscreened/unpinned.
- Checked that "Edit Task" doesn't allow pinning for "Description", etc.
- Pressed "?", read about pressing "Z".
- Pressed "Z".
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T12049
Differential Revision: https://secure.phabricator.com/D17105
Summary: More obvious crumbs, remove border on images, better image spacing.
Test Plan: Build out a test blog, click on crumbs, view spacing.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D17101
Summary: Reorgaizes the CSS here a bit, by object list style, adds in a new drag ui class, which will be used in menu ordering.
Test Plan:
Workboards, Home Apps.
{F2126266}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D17057
Summary: We're currently applying these styles to labels, restrict them to only list elements that have an href.
Test Plan: Test a label, an anchor, and a button as an action item.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D17031
Summary: Fixes T11990. Chrome/Ubuntu doesn't respect `outline: auto`, so this is the only workaround I could fine. Somewhat nicer than stock Chrome.
Test Plan: tab tab tab tab, ubuntu, chrome, windows, safari.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T11990
Differential Revision: https://secure.phabricator.com/D17023
Summary: Use one set of spacing everywhere for action menus
Test Plan: Review dropdowns, action lists.
Reviewers: epriestley, tyhtest
Reviewed By: tyhtest
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D17006
Summary: Form inputs with just text or certain selects had extra space. This uses better text centering.
Test Plan: Review forms on all Settings pages, Custom Policy, Herald.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D17005
Summary:
Persona is going to be decommed November 30th, 2016.
It is highly unlikely that anyone is currently using persona as a real
login method at this point.
Test Plan: tried locally to add auth adapter.
Reviewers: chad, #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: epriestley
Differential Revision: https://secure.phabricator.com/D16371
Summary: Ref T3612. Hides badges on the comment panel.
Test Plan: Give myself a badge, leave a comment, see no badge UI.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T3612
Differential Revision: https://secure.phabricator.com/D16979
Summary: Ref T3612, prevents lightbox from spawning from inside a lightbox.
Test Plan: Click on file lightbox, leave file comment, click file comment, get take to file page instead of another lightbox.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T3612
Differential Revision: https://secure.phabricator.com/D16978
Summary: Ref T3612, this adds a anchor around the large icon with hover state so you can download from here as well.
Test Plan: Hover over .ics file, click, get download.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T3612
Differential Revision: https://secure.phabricator.com/D16977
Summary: Fixes T11791. We do this in durable column, but not in regular Conpherence. I think this is the right place? Not sure how this will feel with high lag.
Test Plan: Submit lots of text in a Conpherence.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T11791
Differential Revision: https://secure.phabricator.com/D16969
Summary: This is still reasonably functional and useful to people, and we don't have better mechanics to offset the change.
Test Plan: New Workboard, set Workboard color, test mobile, desktop.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D16964
Summary: Ref T3612. Mobilizes the new lightbox, changes large buttons to circle icons like Conpherence.
Test Plan: Click each new button on desktop, mobile.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T3612
Differential Revision: https://secure.phabricator.com/D16961
Summary: Ref T3612. Moves the listener to the frame of the image.
Test Plan: Click on image, no close, click on grey frame, closes image. Test image and document, clicking on arrows.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T3612
Differential Revision: https://secure.phabricator.com/D16959
Summary: Ref T3612. Passes in file size and file icon for non-images.
Test Plan: Review a PDF and PSD in a lightbox.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T3612
Differential Revision: https://secure.phabricator.com/D16957
Summary: Spruce up the file embeds a little more, hover state, icons, file size.
Test Plan:
Add a psd and pdf, see new icons. Check differential, still see icons there too. Test mobile, desktop.
{F2042539}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D16950
Summary: Removes the viewable restriction on embedded files. Builds a basic lightbox UI for commenting.
Test Plan:
Add psd, pdf to Maniphest task, clicked on download, comment, left comment. Closed box.
{F1943726}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T3612
Differential Revision: https://secure.phabricator.com/D16917
Summary: Adds a comment box, you can put text into it, hit enter, and see it come back.
Test Plan: Put text into box, see it come back.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T3612
Differential Revision: https://secure.phabricator.com/D16907
Summary: Basic work in progress, but should show timeline comments for files when in lightbox mode. Looks reasonable.
Test Plan: click on images, see comments from timeline.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T3612
Differential Revision: https://secure.phabricator.com/D16896
Summary:
Ref T8510. Use "\n" as a delimiter between name sections. Specifically, project "AAA" with tag "zzz" should be a better match for query "AAA" than project "AAA BBB" is.
Make use of this delimiter slighlty more obvious in the UI.
Test Plan:
- Created projects "Phacility" and "Phacility Core Access".
- Typed "Phacility".
- Before patch: first hit is "Phacility Core Access".
- After patch: first hit is "Phacility".
- Viewed debugging output table, saw visual explanation of behavior.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T8510
Differential Revision: https://secure.phabricator.com/D16886
Summary:
Fixes T11886. D16882 prevented workboard cards from being dragged between columns because it reduced the effective document height to almost nothing (~78px, just the menu bars).
Instead, use the larger of the document height and viewport height as "infinity".
Test Plan: Dragged columns between workboards successfully.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T11886
Differential Revision: https://secure.phabricator.com/D16885
Summary:
Fixes T11785. Lightbox calls `JX.Stratcom.pass()` to let other handlers react, but should not. At least today, we never put, e.g., links inside a lightbox.
This code appears in the original commit so it was probably just copy/pasted from somewhere and I missed it in review.
(Or there's some edge case I'm not thinking of and we'll figure it out soon enough.)
Additionally, blacklist `/file/data/` from Quicksand naviagtion: Quicksand should never fetch these URIs.
Test Plan:
- Disabled `security.alternate-file-domain`.
- Enabled Quicksand ("Persistent Chat").
- Clicked an image thumbnail on a task.
- Repeated that until things flipped out a bit.
- After the patch: no issues.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T11785
Differential Revision: https://secure.phabricator.com/D16884
Summary: Visually, I think these are much cleaner (with colors), and provide more workspace. I also don't really use the sidenav here and if I did, it would be to go back to the project homepage. I think this is overall better. If navigation page to project home is difficult or hard to find, we can maybe make a better header / crumbs bar to reduce that.
Test Plan: New project -> basic new board. Existing project -> color board. Desktop, Mobile, Fullscreen
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D16882
Summary: Fixes some outlying issues with new comment box and Phame Posts.
Test Plan: Review CSS in sandbox, post new reply, see proper spacing.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D16869
Summary:
Ref T11816. We currently try to expand the picker control from the little calendar icon. This works alright on desktop, but not great on tablet/mobile.
On tablet/mobile, center the control on screen instead. Also, mask the background since it can get a bit busy because we can't really control what's under the control anymore.
Finally, move the little popup thing around if the user resizes the window.
Test Plan: {F1922773}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T11816
Differential Revision: https://secure.phabricator.com/D16866
Summary:
Ref T11853. My CSS change for the more enormous policy dialog was a little too broad, and affected the "You shall not pass!" dialog too.
Narrow the scope of the CSS rules.
Also add a missing "." that I caught.
Test Plan:
- Looked at policy exception dialogs.
- Looked at policy explanation dialogs.
- Looked at the end of that sentence.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T11853
Differential Revision: https://secure.phabricator.com/D16841
Summary: On really wide selects, text will go over the background image. Give it more padding.
Test Plan: Review selects on email preferences page.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D16842
Summary: Some aftermath fallout from rebuilding the comment box.
Test Plan: Go fullscreen, pop back out.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D16836
Summary:
Fixes T11836. See some prior discussion in T8376#120613.
The policy hint in headers in the UI is not exhaustive, and can not reasonably be exhaustive. For example, on a revision, it may say "All Users", but really mean "All users who can see the space this object is in and the repository it belongs to, plus the revision author and reviewers".
These rules are explained if you click (and, often, in the documentation), but "All Users" is still at least somewhat misleading.
I don't think there's any perfect solution here that balances the needs of both new and experienced users perfectly, but this change tries to do a bit better about avoiding cases where we say something very open (like "All Users") when the real policy is not very open.
Specifically, I've made these changes to the header:
- Spaces are now listed in the tag, so it will say `(S3 > All Users)` instead of `(All Users)`. They're already listed in the header, this just makes it more explicit that Spaces are a policy container and part of the view policy.
- Extended policy objects are now listed in the tag, so it will say `(S3 > rARC > All Users)` for a revision in the Arcanist repository which is also in Space 3.
- Objects can now provide a "Policy Codex", which is an object that represents a rulebook of more sophisticated policy descriptions. This codex can replace the tag with something else.
- Imported calendar events now say "Uses Import Policy" instead of, e.g., "All Users".
I've made these changes to the policy dialog:
- Split it into more visually separate sections.
- Added an explicit section for extended policies ("You must also have access to these other objects: ...").
- Broken the object policy rules into a "Special Rules" section (for rules like "you can only see a revision if you can see the repository it is part of") and an "Object Policy" section (for the actual object policy).
- Tried to make it a little more readable?
- The new policy dialogs are great to curl up with in front of a fire with a nice cup of cocoa.
I've made these changes to infrastructure:
- Implementing `PhabricatorPolicyInterface` no longer requires you to implement `describeAutomaticCapability()`.
- Instead, implement `PhabricatorPolicyCodexInterface` and return a `PhabricatorPolicyCodex` object.
- This "codex" is a policy rulebook which can set all the policy icons, labels, colors, rules, etc., to properly explain complex policies.
- Broadly, the old method was usually either not useful (most objects have no special rules) or not powerful enough (objects with special rules often need to do more in order to explain them).
Test Plan:
{F1912860}
{F1912861}
{F1912862}
{F1912863}
Reviewers: chad
Reviewed By: chad
Subscribers: avivey
Maniphest Tasks: T11836
Differential Revision: https://secure.phabricator.com/D16830
Summary: Redesign the action comment box for better use in two column, mobile, nuance.
Test Plan: Test in mobile / desktop / tablet, adding and removing actions. Actionless comment boxes, etc.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: cspeckmim, Korvin
Differential Revision: https://secure.phabricator.com/D16811
Summary:
Ref T8510. We had two issues with mixed-case result sorting, like typing `@joe` to match user `Joe`.
- The fallback sort was not normalized properly, so "J" could sort after "j". Instead, normalize values for sorting.
- The `prefix_hits` and older `priority_hits` mechanisms were competing destructively. The `prefix_hits` mechanism completely replaces the `priority_hits` mechanism. Instead, use only the `prefix_hits` mechanism.
Test Plan:
- Copied results for "joe" from WMF.
- Hard-coded the controller to return them.
- Searched for `@joe`.
- After patches, first hit is user "Joe".
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T8510
Differential Revision: https://secure.phabricator.com/D16826
Summary:
Ref T7643. When you do something like this:
- Edit a task description.
- Click "Show Details" on the resulting transaction.
- Get a prose diff dialog showing the change.
...now add some "Old" and "New" tabs. These are useful for:
- reverting to the old text by copy/pasting;
- reading just the new/old text if the diff is noisy;
- sometimes just nice to have?
(This looks a little rough but I didn't want to put a negative margin on tab groups inside dialogs? Not sure what the best fix here is.)
Test Plan: {F1909390}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T7643
Differential Revision: https://secure.phabricator.com/D16817
Summary: Ref T11809. These have been replaced with more flexible storage that accommodates a wider range of behaviors, including those in the ICS format and RRULEs.
Test Plan:
- Ran migration.
- Viewed, created, edited events.
- Grepped for all removed names/symbols.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T11809
Differential Revision: https://secure.phabricator.com/D16789
Summary:
This feels a little cleaner:
- Clean up transaction log a bit.
- Use a checkbox instead of a two-option dropdown.
This is a little messy because the browser doesn't send anything if the user submits a form with an un-clicked checkbox.
We now send a dummy value ("Hey, there's definitely a checkbox in this form!") so the server can figure out what to do.
Test Plan:
- Edited all-dayness of an event.
- Viewed transaction log.
Reviewers: chad
Reviewed By: chad
Differential Revision: https://secure.phabricator.com/D16776
