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

Unified login: Hide TOS buttons if not in signup mode during login #2845

Merged
merged 1 commit into from
Sep 14, 2020

Conversation

AmandaRiu
Copy link
Contributor

This PR closes #2837 by hiding the Terms of Service buttons when not in a "Signup" flow. WooCommerce app does not currently support signup during login.

Before After
Screenshot_1600087617 Screenshot_1600087642

To Test

  1. Click Continue with WordPress.com
  2. Notice the TOS text is not visible in the email form view.

Update release notes:

  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

@AmandaRiu AmandaRiu added this to the 5.1 milestone Sep 14, 2020
@AmandaRiu AmandaRiu requested a review from a team September 14, 2020 12:52
@peril-woocommerce
Copy link

peril-woocommerce bot commented Sep 14, 2020

Fails
🚫

Danger failed to run /app/danger-0.hcbz7xzj4gp.ts.

Messages
📖

This PR contains changes in the subtree libs/login/. It is your responsibility to ensure these changes are merged back into wordpress-mobile/WordPress-Login-Flow-Android. Follow these handy steps!
WARNING: Make sure your git version is 2.19.x or lower - there is currently a bug in later versions that will corrupt the subtree history!

  1. cd woocommerce-android
  2. git checkout issue/2837-hide-tos
  3. git subtree push --prefix=libs/login/ https://github.com/wordpress-mobile/WordPress-Login-Flow-Android.git merge/woocommerce-android/2845
  4. Browse to https://github.com/wordpress-mobile/WordPress-Login-Flow-Android/pull/new/merge/woocommerce-android/2845 and open a new PR.

Error TypeError

Cannot read property 'diff' of null
TypeError: Cannot read property 'diff' of null
    at checkCommitDiffs (/app/danger-0.hcbz7xzj4gp.ts:43:49)
    at process._tickCallback (internal/process/next_tick.js:68:7)

Dangerfile

38|         if (git === undefined) {
39|             console.log("About to crash due to an error")
40|             console.log("File:", thisFile)
41|             console.log("Danger Object: ", danger)
42| 
---------------------------------------------------^
43|             if (danger !== undefined) {
44|                 console.log("Danger is no longer defined")
45|             }
46|             else {

Generated by 🚫 dangerJS

@AmandaRiu AmandaRiu linked an issue Sep 14, 2020 that may be closed by this pull request
@AmandaRiu AmandaRiu added this to In progress in Unified Login Sep 14, 2020
@peril-woocommerce
Copy link

You can test the changes on this Pull Request by downloading the APK here.

@nbradbury nbradbury self-assigned this Sep 14, 2020
public void onClick(View view) {
Context context = getContext();
if ((context instanceof SignupSheetListener)) {
((SignupSheetListener) context).onSignupSheetTermsOfServiceClicked();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this bug was introduced in this PR, but when the TOS button is clicked it doesn't do anything.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah that's because we don't use it (which is why I'm hiding it). It's used by WPAndroid though which is why it exists.

@nbradbury nbradbury self-requested a review September 14, 2020 17:34
Copy link
Contributor

@nbradbury nbradbury left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:shipit:

@nbradbury nbradbury merged commit 74f422b into feature/unified-login Sep 14, 2020
@nbradbury nbradbury deleted the issue/2837-hide-tos branch September 14, 2020 17:34
@AmandaRiu AmandaRiu mentioned this pull request Sep 15, 2020
1 task
@AmandaRiu AmandaRiu moved this from In progress to Done in Unified Login Sep 15, 2020
@designsimply designsimply added feature: login Related to any part of the log in or sign in flow, or authentication. and removed Login labels May 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature: login Related to any part of the log in or sign in flow, or authentication.
Projects
No open projects
Unified Login
  
Done
Development

Successfully merging this pull request may close these issues.

Unified Login: Hide TOS
3 participants