Paths

Table of Contentst

Differential D17274

Fix a couple of home menu issues for logged-out viewers
ClosedPublic
Actions

Authored by epriestley on Jan 31 2017, 7:38 PM.

Tags

None

Referenced Files

	Unknown Object (File)
	Thu, Apr 25, 1:35 AM

	Unknown Object (File)
	Mon, Apr 22, 12:23 AM

	Unknown Object (File)
	Thu, Apr 11, 9:11 AM

	Unknown Object (File)
	Mon, Apr 8, 9:51 PM

	Unknown Object (File)
	Thu, Apr 4, 5:38 PM

	Unknown Object (File)
	Thu, Apr 4, 9:01 AM

	Unknown Object (File)
	Thu, Apr 4, 5:51 AM

	Unknown Object (File)
	Mon, Apr 1, 3:56 PM

Subscribers

None

Details

Reviewers

Maniphest Tasks

T12174: Home Menu Errata

Commits

rP27a33896ff1c: Fix a couple of home menu issues for logged-out viewers

Summary

Ref T12174. These items could fatal ($item not defined) if the viewer was not logged in.

Test Plan

Viewed home as a logged-out user.

Diff Detail

Repository

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

epriestley created this revision.Jan 31 2017, 7:38 PM

Harbormaster completed remote builds in B15455: Diff 41557.Jan 31 2017, 7:39 PM

chad accepted this revision.Jan 31 2017, 7:40 PM

This revision is now accepted and ready to land.Jan 31 2017, 7:40 PM

epriestley added inline comments.Jan 31 2017, 7:40 PM

src/applications/home/menuitem/PhabricatorHomeLauncherProfileMenuItem.php
51–59	oops this should be `!` hue hue

I didn't think to check logged out homepage.

Actually, I guesss "More Applications" is fine to show whether the user is logged in or not.

Show "More Applications" even if the user is not logged in, it's fine for them to go explore Paste or whatever.

Harbormaster completed remote builds in B15456: Diff 41558.Jan 31 2017, 7:43 PM

Closed by commit rP27a33896ff1c: Fix a couple of home menu issues for logged-out viewers (authored by epriestley, committed by epriestley). · Explain WhyJan 31 2017, 7:43 PM

This revision was automatically updated to reflect the committed changes.

What's the plan with mobile

I liked the old behavior (show menu by default). I don't think this behavior (show default item by default) is awful if you want to keep it, but I'd plan to swap back to the old behavior unless you particularly like this one.

I prefer the menu, mostly because I use Feed more on mobile.

Revision Contents
Changeset List

Path

Size

src/

applications/

home/

controller/

PhabricatorHomeMenuItemController.php

8 lines

menuitem/

PhabricatorHomeLauncherProfileMenuItem.php

16 lines

PhabricatorHomeProfileMenuItem.php

20 lines

Diff 41559

src/applications/home/controller/PhabricatorHomeMenuItemController.php

Loading...

src/applications/home/menuitem/PhabricatorHomeLauncherProfileMenuItem.php

Loading...

src/applications/home/menuitem/PhabricatorHomeProfileMenuItem.php

Loading...