Automattic / wp-calypso

The JavaScript and API powered WordPress.com
https://developer.wordpress.com
GNU General Public License v2.0
12.41k stars 1.98k forks source link

Login: post-login pages should display the login form for logged-out users #23785

Closed rralian closed 3 years ago

rralian commented 6 years ago

We should have a framework-level approach to handling logged-out users who load a route requiring login. Right now we display a blank page with links to "sign up" and "log in" in the top bar.

logged out view

But users interpret this as a broken/blank page.

There's a PR to address this for a particular route here #23548.

We were just discussing in slack that this should probably be handled at the nginx level. There are other routes where users wind up seeing these blank pages, for example when we link them to view their receipts from wpchrg.com. It should probably be the default for post-login content to be handled this way, rather than something we need to code specially for each route. I don't think we ever want users to see that blank page.

Assigning to @blowery to figure out how to properly assign to team @Automattic/team-calypso

rachelmcr commented 6 years ago

Some related issues: https://github.com/Automattic/wp-calypso/issues/23623, https://github.com/Automattic/wp-calypso/issues/22857, https://github.com/Automattic/wp-calypso/issues/23033

designsimply commented 6 years ago

Added the [Pri] High label because this issue has been referenced multiple times and because the masterbar is hidden while editing, logged-out views for pages such as https://wordpress.com/post/ show up as a completely white screen with no navigation.

screen shot 2018-06-06 at wed jun 6 2 25 53 pm Seen at https://wordpress.com/post using Firefox 60.0.1 on macOS 10.13.4.

rralian commented 6 years ago

@sirbrillig can you take a look at this? I thought we had addressed this, but I don't remember if that was done for specific routes or at the framework level. Can we use the same approach for high priority routes if that's faster than waiting for a framework-level fix?

sirbrillig commented 6 years ago

@rralian we had fixed it for the purchases routes in #24435, but this issue highlights the situation for the whole of Calypso, which is not so easy to solve since there is no top-level router. The technique can certainly be used on pretty much any route though (my version was simply copied from earlier work in #22993, and @taggon used the same sort of thing in #24687).

I could work on trying to come up with a framework level solution but it's pretty architecture-heavy work and I don't know if I'm going to have the time with my current priorities. I'll chat with you about it on Slack. In the mean time, to whoever might read this next, feel free to use the above techniques for other routes.

rachelmcr commented 6 years ago

I tested and confirmed this is still happening as described above. Is this still considered a high priority issue? It feels very broken when logged-out users are directed to logged-in pages, but given there's a workaround (fixing it for specific routes), I could see lowering the priority on this issue if there are other priorities for framework/high-level work right now.

blowery commented 6 years ago

I think we can drop the priority here.

We've spun through this a couple times and it's complicated and fragile to solve generically with page.js. The only way to add a 404 handler is by attaching a * route handler as the last handler overall. However, we add routes on the fly when async loading code...

blowery commented 6 years ago

Sticking it on @Automattic/team-calypso's radar again, even with the dropped priority.

github-actions[bot] commented 3 years ago

This issue is stale because it has been 180 days with no activity. You can keep the issue open by adding a comment. If you do, please provide additional context and explain why you’d like it to remain open. You can also close the issue yourself — if you do, please add a brief explanation and apply one of relevant issue close labels.

jsnajdr commented 3 years ago

This has been solved over the last few years by consistently marking which pages are intended for logged-out users and redirecting all other pages to login page.