Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Login: Update route to use /me/login #14144

Closed
wants to merge 3 commits into from
Closed

Conversation

drewblaisdell
Copy link
Contributor

This is necessary to prevent conflicts with /login, which points to the old login page.

Testing

  • Visit /.

  • Click the login button.

  • Assert that you are taken to /me/login?redirect_to=….

  • Assert that you can log in as before.

  • Code

  • Product

This is to prevent conflicts with `/login`.
@drewblaisdell drewblaisdell added Login [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. labels May 17, 2017
@drewblaisdell drewblaisdell self-assigned this May 17, 2017
@matticbot
Copy link
Contributor

@matticbot matticbot added the [Size] S Small sized issue label May 17, 2017
@stephanethomas
Copy link
Contributor

I've just fixed the following error that was triggered when starting the server:

{
  "formatted": "Error: File to import not found or unreadable: login/wp-login/style\n       Parent style sheet: /var/sources/assets/stylesheets/_components.scss\n        on line 21 of assets/stylesheets/_components.scss\n>> @import 'login/wp-login/style';\n   ^\n",
  "message": "File to import not found or unreadable: login/wp-login/style\nParent style sheet: /var/sources/assets/stylesheets/_components.scss",
  "column": 1,
  "line": 21,
  "file": "/var/sources/assets/stylesheets/_components.scss",
  "status": 1
}

@stephanethomas
Copy link
Contributor

The code looks good. I'm not convinced that we should move the login code under /client/me though. We decided to move the new Login page to https://wordpress.com/me/login instead of https://wordpress.com/login to avoid making a system request, and to be able to run both the old and the new Login pages side by side. From my understanding, we will remove the former at some point, and move the latter back to https://wordpress.com/login.

@stephanethomas
Copy link
Contributor

The Login page is no longer rendered server-side, something that you can check with Javascript disabled:

screenshot

@gziolo
Copy link
Member

gziolo commented May 17, 2017

Let me fix SSR :)

The Login section should be declared before the Me section as otherwise the latter takes precedence.
@stephanethomas
Copy link
Contributor

I've added a commit that fix server-side rendering. This is good for review.

@stephanethomas
Copy link
Contributor

I just spotted another problem. If you start from http://calypso.localhost:3000/ (which will redirect you to http://calypso.localhost:3000/devdocs/start) or http://calypso.localhost:3000/start and click the Log In link in the top bar you are redirected on the new Login page. However, it's possible to still see part of the content of the previous page at the bottom:

screenshot

@gziolo
Copy link
Member

gziolo commented May 17, 2017

Let's close it and open another PR with /log-in as discussed on Slack to avoid more hidden 🐛 :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Login [Size] S Small sized issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants