Summary:
This could probably use some refinement (and, like, explanatory text, and stronger cues about what rows and columns mean) but feels fairly good to me, at least on test data.
I didn't do any scrolling for now since we have to do full height on mobile anyway I think. I did swap it so the newer ones are on top.
Left/right navigate you among current images only, but you can click any thumb to review history.
Removed history view since it's no longer useful.
Some things that would probably help:
- Some kind of header explaining what this is ("Mock History" or something).
- Stronger visual cue that columns are related by being the same image.
- Clearer cues about obsolete/deleted images (e.g., on the stage itself?)
- Maybe general tweaks.
- Maybe a placeholder (like a grey "X") for images which have been deleted.
(I'm planning to add comment counts too, which I think will be pretty useful, but that felt good to put in another diff.)
Test Plan: See screenshots.
Reviewers: chad
Reviewed By: chad
Subscribers: epriestley
Differential Revision: https://secure.phabricator.com/D9543
Summary: This crumb, which is consistently available in other applications, is not currently available in Pholio.
Test Plan: Viewed an edit page, clicked the crumb.
Reviewers: chad
Reviewed By: chad
Subscribers: epriestley
Differential Revision: https://secure.phabricator.com/D9542
Summary:
This fixes a weird issue which currently doesn't have much impact on things, but starts to matter if we do the grid.
We're incorrectly initializing the form with `replacesPHID` as the //previously replaced Image PHID//. It is supposed to be the //current File PHID//.
Every other time, this is `null` and things work properly. On even updates (2, 4, 6, etc.), it's wrong and we don't record the replacement completely correctly.
Test Plan: Replaced images twice, saw three rows of thumb grid.
Reviewers: chad
Reviewed By: chad
Subscribers: epriestley
Differential Revision: https://secure.phabricator.com/D9541
Summary:
Currently, we limit the image size to make sure that the stage has a constant height and the entire image always fits on screen.
In practice, these don't actually seem like desirable qualities. Instead, focus on giving as many pixels as possible to the image.
Test Plan: See screenshots.
Reviewers: chad
Reviewed By: chad
Subscribers: epriestley
Differential Revision: https://secure.phabricator.com/D9539
Summary:
- Moves the right-hand gutter under the image.
- Moves size information to the upper right.
- This is transitional, on the way toward something more like the mocks in D9534.
Test Plan: See screenshot.
Reviewers: chad
Reviewed By: chad
Subscribers: epriestley
Differential Revision: https://secure.phabricator.com/D9538
Summary: This greatly simplifies inline comments while retaining their functionality. This is probably not where we want to end up, but will let us figure out what we're doing with the stage without worrying about inlines.
Test Plan: See screenshots.
Reviewers: chad
Reviewed By: chad
Subscribers: epriestley
Differential Revision: https://secure.phabricator.com/D9537
Summary:
- Hide inline areas on mobile, and when the mouse cursor is not on the stage.
- Show small icon markers instead of areas.
- Yellow icons show draft comments; pink icons show final comments.
Test Plan: {F166544}
Reviewers: chad
Reviewed By: chad
Subscribers: epriestley
Differential Revision: https://secure.phabricator.com/D9536
Summary: Uses common CSS spacing, tweaks some colors, increases viewport size to very limits.
Test Plan:
Test large and small images, various breakpoints. Able to more easily review mocks.
{F166500}
{F166501}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D9534
Summary: Changes the old dark embed to match the PinboardView. Retains ability to target individual files. Removes "carousel" of files (not super useful?)
Test Plan:
Tested embedding Mocks, with and without targeting specific files. Tested Pholio Pinboard, Macro Pinboard.
{F166451}
{F166452}
{F166453}
{F166454}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D9531
Summary: As pointed out by @epriestley in D9458#62, this call to `debug.log` is missing an argument.
Test Plan: meh..
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D9485
Summary: When you currently create a Pholio Mock, it's closed if you didn't notice the select. This hides the input.
Test Plan: Created a Pholio Mock, verified it was open
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D9522
Summary: It should have the consistant 8px gutter.
Test Plan: Reload profile at mobile breakpoints.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D9521
Summary: Not sure if this is the correct fix, but I think it's where you intend to go?
Test Plan: Click on link in Task, get the the correct board. Click lots of links of boards and make sure everything still works.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Maniphest Tasks: T5361
Differential Revision: https://secure.phabricator.com/D9520
Summary: Clear the float, remove the abs. position.
Test Plan:
Test a project hovercard, see expected layout.
{F166175}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D9519
Summary: The CSS rule tends to miss many tables, make the rule more universal and add borders as needed.
Test Plan: Test a Revision and Diffusion
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D9516
Summary: We apply a mobile button style here, make sure to only use it in Property Lists, and not all PHUIX dropdowns.
Test Plan: Test dropdown on mobile
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D9517
Summary: This implements showing the buildable status in Diffusion and unifies some of the logic used to calculate and render build and buildable statuses.
Test Plan: Looked at diffs and commits with statuses, they rendered fine. Looked at Diffusion and saw buildable status appear (with a manual buildable and manual buildables included in the query).
Reviewers: #blessed_reviewers, epriestley, chad
Reviewed By: #blessed_reviewers, epriestley
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D9496
Summary:
Via HackerOne. There are two attacks here:
- Configuring mirroring to a `file://` URI to place files on disk or overwrite another repository. This is not particularly severe.
- Configuring cloning from a `file://` URI to read repositories you should not have access to. This is more severe.
Historically, repository creation and editing explicitly supported `file://` URIs to deal with use cases where you had something else managing repositories on the same machine. Since there were no permissions, repository management was admin-only, and you couldn't mirror, this was fine.
As we've evolved, this use case is a tiny minority use case and the security implications of `file://` URIs overwhelm the utility it provides. Prevent the use of `file://` URIs. Existing configured repositories won't stop working, you just can't add any new ones.
Also prevent `localPath` from being set via Conduit (see T4039).
Test Plan:
- Tried to create a `file://` repository.
- Tried to create a `file://` mirror.
- Tried to create a `file://` repository via Conduit.
- Created a non-`file://` repository.
- Created a non-`file://` mirror.
- Created a non-`file://` repository via Conduit.
Reviewers: btrahan, chad
Reviewed By: chad
Subscribers: epriestley
Differential Revision: https://secure.phabricator.com/D9513
Summary: This UI recommends `bin/remove destroy X`, but should recommend `bin/remove destroy rX` (with `r`), because the remove script now takes any object monogram. The older script was repository-specific, so it only took the callsign.
Test Plan: {F166042}
Reviewers: putnam, chad
Reviewed By: chad
Subscribers: epriestley
Differential Revision: https://secure.phabricator.com/D9512
Summary: This ensure that Aphlict is always served on the primary / production domain, because the alternate file domain is intended to prevent Flash execution.
Test Plan: Confirmed this fixes the issue by visiting https://code.redpointsoftware.com.au/ and seeing notification messages.
Reviewers: joshuaspence, #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D9511
Summary:
Further improve UX for dealing with policy rules on dashboards:
- When in the "Manage" view of a dashboard you can not edit:
- Don't show the panel management controls.
- Show a notice that the board isn't editable, recommending you make a copy instead.
- Add a "Copy Dashboard" action to create a copy which you //can// edit.
Test Plan: Copied some dashboards. See screenshots.
Reviewers: chad
Reviewed By: chad
Subscribers: epriestley
Differential Revision: https://secure.phabricator.com/D9508
Summary: Fixes T5320. Adds a "Home" application at the top, for mobile-only.
Test Plan: See screenshots.
Reviewers: chad
Reviewed By: chad
Subscribers: epriestley
Maniphest Tasks: T5320
Differential Revision: https://secure.phabricator.com/D9509
Summary: Fix the URL to editing columns, fix the color of a PHUIX dropdown(simple)
Test Plan: Click on Dropdown, don't feel offended. Edit a Column from various search URLs. Fixes T5341
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Maniphest Tasks: T5341
Differential Revision: https://secure.phabricator.com/D9507
Summary: Adds some basic links to Project ObjectItems, Workboards and Members. Assume these will be configurable by CustomFields off in the future, but this makes Projects on Dashboards much more useful.
Test Plan:
Tested /projects/ and /dashboards/, click on links to make sure they go where expected.
{F164972}
{F164973}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D9483
Summary:
Fixes T5167. When clicking "Edit" on a dashboard panel you don't own, the UI now allows you to make a copy instead.
As a bonus, fixes T5259.
Test Plan: See screenshots.
Reviewers: chad
Reviewed By: chad
Subscribers: epriestley
Maniphest Tasks: T5259, T5167
Differential Revision: https://secure.phabricator.com/D9505
Summary:
Fixes T5308.
- Allows you to create a panel directly on a dashboard.
- Also, include existing panels with a select instead of a text field. This won't scale as well but should be fine for now, and is way easier to use.
Test Plan: See comment.
Reviewers: chad
Reviewed By: chad
Subscribers: epriestley
Maniphest Tasks: T5308
Differential Revision: https://secure.phabricator.com/D9501
Summary: Ref T4986. Instead of requiring users to know the name of an application search engine class, let them select from a list.
Test Plan:
Created a new panel.
{F165468}
Reviewers: chad
Reviewed By: chad
Subscribers: epriestley
Maniphest Tasks: T4986
Differential Revision: https://secure.phabricator.com/D9500
Summary: When you "Edit Panel" on a dashboard, pop a dialog instead of redirecting to a different page.
Test Plan: Edited a panel from a dashboard; edited a panel from the panel workflow.
Reviewers: chad
Reviewed By: chad
Subscribers: epriestley
Differential Revision: https://secure.phabricator.com/D9499
Summary: Adds some minor tweaks to the Quoted Remarkup rules. I tried altering the nested quote as well, but it became a bit of a joke quickly, not sure there is benefit there. Fixes T4962
Test Plan:
Quoted lots of times.
{F165606}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Maniphest Tasks: T4962
Differential Revision: https://secure.phabricator.com/D9502
Summary: Moves the icon into the link
Test Plan: Tested a form (Phrequent), disabled, and regular action link
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Maniphest Tasks: T5331
Differential Revision: https://secure.phabricator.com/D9503
Summary: Nothing inside Phabricator uses the return value of this method, but returning the actual build instance is far more useful (for kicking off builds in an application and storing the build PHID against another object).
Test Plan: N/A
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D9494
Summary: Ref T5284. When an `AphlictClient` is closed, it will eventually be purged from the pool by the `AphlictMaster`. When this happens, also unsubscribe the purged client from all notifications, and send an `unsubscribe` command to the Aphlict server if possible.
Test Plan:
Verified the output of the Aphlict server (running in debug mode):
```
[Wed Jun 11 2014 23:21:31 GMT+0000 (UTC)] <FlashPolicy> Policy Request From ::ffff:192.168.1.1
[Wed Jun 11 2014 23:21:31 GMT+0000 (UTC)] <Listener/2> Connected from ::ffff:192.168.1.1
[Wed Jun 11 2014 23:21:31 GMT+0000 (UTC)] <Listener/2> Received data: {"command":"subscribe","data":["PHID-USER-cb5af6p4oepy5tlgqypi"]}
[Wed Jun 11 2014 23:21:31 GMT+0000 (UTC)] <Listener/2> Subscribed to: ["PHID-USER-cb5af6p4oepy5tlgqypi"]
[Wed Jun 11 2014 23:21:39 GMT+0000 (UTC)] <Listener/2> Received data: {"command":"subscribe","data":["PHID-TASK-l2dtbs5xrt2b7abgh5a6"]}
[Wed Jun 11 2014 23:21:39 GMT+0000 (UTC)] <Listener/2> Subscribed to: ["PHID-TASK-l2dtbs5xrt2b7abgh5a6"]
[Wed Jun 11 2014 23:21:57 GMT+0000 (UTC)] <Listener/2> Received data: {"command":"unsubscribe","data":["PHID-TASK-l2dtbs5xrt2b7abgh5a6"]}
[Wed Jun 11 2014 23:21:57 GMT+0000 (UTC)] <Listener/2> Unsubscribed from: ["PHID-TASK-l2dtbs5xrt2b7abgh5a6"]
```
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: epriestley, Korvin
Maniphest Tasks: T5284
Differential Revision: https://secure.phabricator.com/D9492
Summary: Makes the code view have a border, proper spacing.
Test Plan: view a list of pastes
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D9491
Summary: These should be thin, not light
Test Plan: reload html
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D9490
Summary: Currently, the `AphlictClient` will only send its subscriptions to the `AphlictMaster` once. If the original `AphlictMaster` is closed and a new master is created, then client subscriptions will be lost.
Test Plan: Opened two separate tabs. Closed the "master" tab and noticed that the subscriptions were re-sent to the server.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D9487
Summary:
Ref T4083. This needs some work (mostly in the Conpherence JS itself), but is sort of functional. In particular:
- On thread pages, add the thread as a `pageObject`.
- After updating a thread, send a new "message" event to the server.
- Share a little more event posting code.
- In the browser, use event dispatch to respond to events.
- Add a listener for the new event type.
- Update conpherence threads (this part is really yucky).
Test Plan: With multiple browser windows / browsers open, posted a message to a thread, and saw it update everywhere.
Reviewers: joshuaspence
Reviewed By: joshuaspence
Subscribers: chad, epriestley
Maniphest Tasks: T4083
Differential Revision: https://secure.phabricator.com/D9486
Summary: Ref T4324. Ref T5284. This adds server-side support for keeping track of a set of PHIDs that the Aphlict clients have subscribed to. Instead of broadcasting a notification to all clients (after which the clients can poll `/notification/individual` in order to determine whether or not they are interested in the notification), transmit notifications only to clients that have subscribed to a PHID that is relevant to the notification.
Test Plan:
I opened up two clients on the same host (incognito tabs in Chrome). Here is the output from the server:
```
> sudo ./bin/aphlict debug
Starting Aphlict server in foreground...
Launching server:
$ 'nodejs' '/usr/src/phabricator/src/applications/aphlict/management/../../../../support/aphlict/server/aphlict_server.js' --port='22280' --admin='22281' --host='localhost' --user='aphlict'
[Wed Jun 11 2014 19:10:27 GMT+0000 (UTC)] Started Server (PID 4546)
[Wed Jun 11 2014 19:10:36 GMT+0000 (UTC)] <FlashPolicy> Policy Request From ::ffff:192.168.1.1
[Wed Jun 11 2014 19:10:37 GMT+0000 (UTC)] <Listener/1> Connected from ::ffff:192.168.1.1
[Wed Jun 11 2014 19:10:37 GMT+0000 (UTC)] <Listener/1> Received data: {"command":"subscribe","data":["PHID-USER-cb5af6p4oepy5tlgqypi"]}
[Wed Jun 11 2014 19:10:37 GMT+0000 (UTC)] <Listener/1> Subscribed to: ["PHID-USER-cb5af6p4oepy5tlgqypi"]
[Wed Jun 11 2014 19:10:39 GMT+0000 (UTC)] <Listener/1> Received data: {"command":"subscribe","data":["PHID-USER-kfohe3ca5oe6ygykmioq"]}
[Wed Jun 11 2014 19:10:39 GMT+0000 (UTC)] <Listener/1> Subscribed to: ["PHID-USER-kfohe3ca5oe6ygykmioq"]
[Wed Jun 11 2014 19:10:42 GMT+0000 (UTC)] notification: {"key":"6023751084283587681","type":"notification","subscribers":["PHID-USER-cb5af6p4oepy5tlgqypi"]}
[Wed Jun 11 2014 19:10:42 GMT+0000 (UTC)] <Listener/1> Wrote Message
```
I verified (using the "Network" tab in Chrome) that an AJAX request to `/notification/individual/` was only made in the tab belonging to the user that triggered the test notification.
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: epriestley, Korvin
Maniphest Tasks: T5284, T4324
Differential Revision: https://secure.phabricator.com/D9458
Summary:
Remove's PhabricatorBotDifferentialNotificationHandler documentation and adds in:
PhabricatorBotFeedNotificationHandler
PhabricatorBotSymbolHandler
PhabricatorBotMacroHandler
Should have been included in D9477
Test Plan: Read it..?
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D9479
Summary: Printing out `err` is less informative than `err.stack`, which has the message, type, //and// a trace.
Test Plan:
Faked an exception, then:
```
$ sudo ./bin/aphlict debug
Starting Aphlict server in foreground...
Launching server:
$ 'node' '/INSECURE/devtools/phabricator/src/applications/aphlict/management/../../../../support/aphlict/server/aphlict_server.js' --port='22280' --admin='22281' --host='localhost'
[Wed Jun 11 2014 10:20:39 GMT-0700 (PDT)]
<<< UNCAUGHT EXCEPTION! >>>
Error: Example Exception
at Object.<anonymous> (/INSECURE/devtools/phabricator/support/aphlict/server/aphlict_server.js:73:7)
at Module._compile (module.js:456:26)
at Object.Module._extensions..js (module.js:474:10)
at Module.load (module.js:356:32)
at Function.Module._load (module.js:312:12)
at Function.Module.runMain (module.js:497:10)
at startup (node.js:119:16)
at node.js:901:3
>>> Server exited!
```
Reviewers: joshuaspence
Reviewed By: joshuaspence
Subscribers: epriestley
Differential Revision: https://secure.phabricator.com/D9481
Summary: Adds a hover state and color for white headers. Fixes T5318
Test Plan: Test on managing a dashboard.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Maniphest Tasks: T5318
Differential Revision: https://secure.phabricator.com/D9482
Summary: Fixes T5271. This is mostly similar to normal tab panel JS, but I think we'll eventually do async rendering and/or saved tabs so it's reasonable to split it out.
Test Plan: Toggled tabs on a tab panel, saw tab selected state change.
Reviewers: btrahan, chad
Reviewed By: chad
Subscribers: epriestley
Maniphest Tasks: T5271
Differential Revision: https://secure.phabricator.com/D9478
Summary:
Currently, the Aphlict server will crash if invalid JSON data is `POST`ed to it. I have fixed this to, instead, return a 400. Also made some minor formatting changes.
Ref T4324. Ref T5284. Also, modify the data structure that is passed around (i.e. `POST`ed to the Aphlict server and broadcast to the Aphlict clients) to include the subscribers. Initially, I figured that we shouldn't expose this information to the clients... however, it is necessary for T4324 that the `AphlictMaster` is able to route a notification to the appropriate clients.
Test Plan:
Making the following `curl` request: `curl --data "{" http://localhost:22281/`.
**Before**
```
sudo ./bin/aphlict debug
Starting Aphlict server in foreground...
Launching server:
$ 'nodejs' '/usr/src/phabricator/src/applications/aphlict/management/../../../../support/aphlict/server/aphlict_server.js' --port='22280' --admin='22281' --host='localhost' --user='aphlict'
[Wed Jun 11 2014 17:07:51 GMT+0000 (UTC)] Started Server (PID 2033)
[Wed Jun 11 2014 17:07:55 GMT+0000 (UTC)]
<<< UNCAUGHT EXCEPTION! >>>
SyntaxError: Unexpected end of input
>>> Server exited!
```
**After**
(No output... the bad JSON is caught and a 400 is returned)
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: epriestley, Korvin
Maniphest Tasks: T4324, T5284
Differential Revision: https://secure.phabricator.com/D9480
Summary: D9468 removed deprecated PhabricatorBotDifferentialNotificationHandler. So, lets remove it from the example json.
Test Plan: Verify json is valid, launch a phabot instance with it
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D9477
