Page MenuHomePhabricator

Use new Guide layout in Config->Welcome
ClosedPublic

Authored by chad on Sep 1 2016, 8:07 PM.
Tags
None
Referenced Files
F13130098: D16482.diff
Wed, May 1, 6:10 AM
Unknown Object (File)
Thu, Apr 25, 12:08 AM
Unknown Object (File)
Tue, Apr 16, 8:30 PM
Unknown Object (File)
Fri, Apr 12, 6:34 AM
Unknown Object (File)
Fri, Apr 12, 6:34 AM
Unknown Object (File)
Fri, Apr 12, 6:34 AM
Unknown Object (File)
Fri, Apr 12, 6:34 AM
Unknown Object (File)
Fri, Apr 12, 6:34 AM
Subscribers

Details

Summary

Ref T11132, swaps in new UI for welcome page using guide modules

Test Plan

Test instance and non instance guides. Test each setting. Unclear on how to test people / Phacility. Just change the URL link?

Diff Detail

Repository
rP Phabricator
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

chad retitled this revision from to Use new Guide layout in Config->Welcome.
chad updated this object.
chad edited the test plan for this revision. (Show Details)
chad added a reviewer: epriestley.

Can we just link to /guides/ instead? Seems like we have two mostly-identical copies of the same page now?

(Code seems fine, I'm just not sure what the distinction/plan is on ConfigWelcome vs Guides -- seems like ConfigWelcome is completely obsolete now?)

I can un-prototype guides and just link to it from Config, I just hadn't thought through all the cases for that - like /welcome/ is the default for guides, but it's written for Phabricator, not Phacility. I think I can make guide modules on.... CORE? and have them pull in? Not sure where Phacility modules would live and get pulled in.

That seems reasonable to me.

Phacility Modules should either go in Services (rSERVICES) ("How to use your new Phabricator instance") or possibly in the far future in Instances (rSAAS) ("How to use the Phacility administration console").

There were also 2 TODOs here, I could use a short reply on. Mostly - "Go check out Applications" seems exceedingly vague, and no real means of verifying success, but still useful to point to the page.

On actual content:

I don't think we should instruct users to uninstall applications. I think this is an unusual workflow (for example, when you install OSX or Windows the first thing you see isn't a prompt to go delete stuff), it feels weird, and it's bad for us and for users. I don't think having applications installed should be hurting anything. The only case where I'd expect installs to actually remove an application is in fairly rare cases to actively prevent users from using it (e.g., Legal is uncomfortable with chat being available in Phabricator or whatever because of international subpoena laws).

In most cases, a large number of Applications should be able to exist without hurting anything. When you install an OS, you get tons of apps and that's fine. Your phone comes with tons of apps too. In the future, installs should be able to add a hundred third-party applications without hurting the experience for users. Although you might go organize your phone a bit at some point, I don't think anyone goes and deletes Calculator.exe after installing Windows because they use Google Calculator instead. If there are cases where having lots of applications is harmful, we should try to fix those problems without requiring users to uninstall applications (D16459 is a small change in this direction).

One such problem case is that the large number of applications can possibly be overwhelming to new users, but I don't think that's inherently a problem of having a lot of applications, and that's what this workflow should fix. This workflow should be able to guide users to accomplish things without requiring them to uninstall applications.

We could alternately test to see if users have configured any application (e.g., changed policies) but I don't think first-time administrators are likely to be able to understand how/why to do this, and especially not as a first step.

We could do something vague ( "Explore Applications") but currently have no way to verify success.

And then, per T11132#191013, I think:

  • We should not put "Invite Collaborators" above content creation. I think first-time administrators are likely to want to create content before they show anyone, and that new invites will have a better experience if they reach an install with content. It's way better to get an invite from a first-time administrator who has already convinced themselves that they like Phabricator, added a repository or two, created a relevant dashboard, and maybe personalized the install, and wants to show that stuff off than an invite from a first-time administrator who isn't familiar with the software yet to join an empty instance that they aren't sold on. "Invite Collaborators" should be near the end: "show off all your work".
  • We should not prompt first-time administrators to create projects. Particularly as written, I think this step will teach the wrong lessons and reinforce an incorrect model for users coming from JIRA.

I'd guess our completion rate on "Build a Dashboard" as written would also be close to 0%. To complete the quest you currently need to create a dashboard and install it on the home page, which we don't tell you explicitly and don't give you any guidance on.

The shortest path I see here without doing engineering NUX work is maybe to turn "Applications" and "Dashboards" into not-auto-detected guidance paragraphs that are just static text with a link to the thing and not completable? Not great, but better than nothing, which is basically what we have today.

The People check is still fine on Phacility in terms of looking for completion (when users accept invitations, accounts get generated), the link to /people/invite/ is the only issue. To fix that I think we have to do more modularization so Phacility can turn off the upstream "invite" quest and put a quest of its own in its place.

chad edited edge metadata.
  • unprototype guides
  • more polish on quickstart
epriestley edited edge metadata.

This looks basically good to me, some general stuff:

Instead of canUseInInstance(), it might be better to have a test like isModuleEnabled(), then just have the "Install Guide" module return false if cluster.instance is set. This is a little more flexible in the future and will let modules disable themselves in other situations (for example, if we add a "Code Review" module, it could disable itself if Differential was not installed; and the "Install" module could disable itself for non-administrators, etc).

Even with the "graph" help text I suspect the "Project" quest might do more harm than good, but I guess we can see what happens.

Not totally obvious to me that I should click "Quick Start" from /guides/, and we link to "Welcome" as "Quick Start" from the feed item. Maybe either link to "Quick Start" directly from the empty-feed story, or link to "Quick Start" explicitly in the intro text for Guides? Not sure.

Linking to /diffusion/, etc., will presumably hit NUX states to help users find "Create", but it might be hard to find the "Invite Users" button from /people/invite/. But linking directly to the invite form could be equally confusing. This is sort of the bigger "how do we lead the users through a navigation pathway" problem, I think.

src/applications/guides/module/PhabricatorGuideQuickStartModule.php
181–183

It's important to me that we not put any Phacility cluster-specific code in the upstream. Instead, this item should just disable itself if cluster.instance is set. We'll provide this in the cluster later through additional modularization of quests.

(Small amounts of grey area stuff like testing cluster.instance are somewhat-OK, but not great. If we were just writing Phacility code into Phabricator a better test would be checking if phabricator.base-uri ends in .phacility.com or PhacilityCore exists. But I don't want this kind of explicitly-Phacility-specific feature in the upstream. And it's impossible to get an instance ID without writing Phacility-specific code.)

(If you don't want to ship this without this step, let's hold it until we can modularize the quests.)

203

"built as graph" missing some words?

This revision is now accepted and ready to land.Sep 2 2016, 11:54 PM
chad edited edge metadata.
  • use getEnabledModules
  • remove welcome, have /guides/ default to first enabled module
This revision was automatically updated to reflect the committed changes.