Page MenuHomePhabricator

Remove skins from Phame
ClosedPublic

Authored by epriestley on Dec 11 2015, 3:19 PM.
Tags
None
Referenced Files
F13275199: D14740.diff
Fri, May 31, 4:31 AM
F13262095: D14740.diff
Mon, May 27, 1:46 AM
F13255564: D14740.id35651.diff
Sat, May 25, 6:13 AM
F13255562: D14740.id35650.diff
Sat, May 25, 6:13 AM
F13251731: D14740.id35652.diff
Sat, May 25, 12:34 AM
F13248334: D14740.id35652.diff
Fri, May 24, 2:32 AM
F13248333: D14740.id.diff
Fri, May 24, 2:32 AM
F13248332: D14740.diff
Fri, May 24, 2:32 AM
Subscribers

Details

Reviewers
chad
Maniphest Tasks
T9897: Unbeta Feedback on Phame
Commits
Restricted Diffusion Commit
rP8a906b0e18a7: Remove skins from Phame
Summary

Ref T9897. Purge a bunch of stuff:

  • Remove skins.
  • Remove all custom sites for skin resources.
  • Remove "framed", "notlive", "preview", separate "live" controllers (see below).
  • Merge "publish" and "unpublish" controllers into one.

New behavior:

  • Blogs and posts have three views:
    • "View": Internal view URI, which is a normal detail page.
    • "Internal Live": Internal view URI which is a little prettier.
    • "External Live": External view URI for an external domain.

Right now, the differences are pretty minor (basically, different crumbs/chrome). This mostly gives us room to put some milder flavor of skins back later (photography or more "presentation" elements, for example).

This removes 9 million lines of code so I probably missed a couple of things, but I think it's like 95% of the way there.

Test Plan

Here are some examples of what the "view", "internal" and "external" views look like for blogs (posts are similar):

"View": Unchanged

Screen Shot 2015-12-11 at 7.11.12 AM.png (1×1 px, 124 KB)

"Internal": No chrome or footer. Still write actions (edit, post commments). Has crumbs to get back into Phame.

Screen Shot 2015-12-11 at 7.11.14 AM.png (1×1 px, 75 KB)

"External": No chrome or footer. No write actions. No Phabricator crumbs. No policy/status information.

Screen Shot 2015-12-11 at 7.11.21 AM.png (1×1 px, 68 KB)

I figure we'll probably tweak these a bit to figure out what makes sense (like: maybe no actions on "internal, live"? and "external, live" probably needs a way to set a root "Company >" crumb?) but that they're reasonable-ish as a first cut?

Diff Detail

Repository
rP Phabricator
Branch
phame1
Lint
Lint Passed
Unit
Tests Passed
Build Status
Buildable 9468
Build 11282: Run Core Tests
Build 11281: arc lint + arc unit

Event Timeline

epriestley retitled this revision from to Remove skins from Phame.
epriestley updated this object.
epriestley edited the test plan for this revision. (Show Details)
epriestley added a reviewer: chad.
epriestley edited edge metadata.
  • Also remove getBlogCDNRoutes(), which is obsolete without skins.
chad edited edge metadata.
This revision is now accepted and ready to land.Dec 11 2015, 3:37 PM
This revision was automatically updated to reflect the committed changes.