Page MenuHomePhabricator

Remove skins from Phame
ClosedPublic

Authored by epriestley on Dec 11 2015, 3:19 PM.
Tags
None
Referenced Files
F13059241: D14740.diff
Fri, Apr 19, 3:55 PM
Unknown Object (File)
Tue, Apr 9, 12:25 PM
Unknown Object (File)
Sat, Apr 6, 10:37 PM
Unknown Object (File)
Thu, Apr 4, 7:40 PM
Unknown Object (File)
Mon, Apr 1, 3:21 PM
Unknown Object (File)
Sat, Mar 30, 1:02 AM
Unknown Object (File)
Sun, Mar 24, 5:36 PM
Unknown Object (File)
Sun, Mar 24, 12:25 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.