Page MenuHomePhabricator

Restore the "Log In" menubar action
ClosedPublic

Authored by epriestley on Dec 5 2017, 1:47 PM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, Dec 20, 1:45 AM
Unknown Object (File)
Wed, Dec 11, 10:26 AM
Unknown Object (File)
Mon, Dec 9, 4:36 PM
Unknown Object (File)
Sun, Dec 8, 11:21 PM
Unknown Object (File)
Sat, Dec 7, 1:52 PM
Unknown Object (File)
Fri, Dec 6, 2:26 PM
Unknown Object (File)
Thu, Dec 5, 3:47 PM
Unknown Object (File)
Thu, Nov 28, 11:22 PM
Subscribers
None

Details

Summary

See https://discourse.phabricator-community.org/t/activation-link-in-welcome-mail-only-works-if-new-user-isnt-semi-logged-in/740/7.

In T13024, I rewrote the main menu bar to hide potentially sensitive items (like notification and message counts and saved search filters) until users fully log in.

However, the "Log In" item got caught in this too. For clarity, rename shouldAllowPartialSessions() to shouldRequireFullSession() (since logged-out users don't have any session at all, so it would be a bit misleading to say that "Log In" "allows" a partial session). Then let "Log In" work again for logged-out users.

(In most cases, users are prompted to log in when they take an action which requires them to be logged in -- like creating or editing an object, or adding comments -- so this item doesn't really need to exist. However, it aligns better with user expectations in many cases to have it present, and some reasonable operations like "Check if I have notifications/messages" don't have an obvious thing to click otherwise.)

Test Plan

Viewed site in an incognito window, saw "Log In" button again. Browsed normally, saw normal menu.

Diff Detail

Repository
rP Phabricator
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

This revision is now accepted and ready to land.Dec 5 2017, 5:02 PM
This revision was automatically updated to reflect the committed changes.