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.

Sign up for GitHub

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

Jump to bottom

Add showTerms option #1485

Merged
merged 9 commits into from
Nov 9, 2018
Merged

Add showTerms option #1485

merged 9 commits into from
Nov 9, 2018

Conversation

luisrudge
Copy link
Contributor

@luisrudge luisrudge commented Sep 18, 2018 •
edited
Loading

Add showTerms option to show/hide terms string in the signup page

@luisrudge luisrudge added this to the 11.9.2 milestone Sep 18, 2018
@cocojoe
Copy link
Member

cocojoe commented Sep 18, 2018

Add description, why are we making this change? Should add screenshots as it will change the default display behaviour with the additional box.

@luisrudge
Copy link
Contributor Author

@cocojoe updated

@luisrudge luisrudge changed the title Adding default signup label [WIP] Adding default signup label Sep 26, 2018
@luisrudge luisrudge removed this from the 11.9.2 milestone Sep 26, 2018
@luisrudge luisrudge mentioned this pull request Oct 3, 2018
Closed
@luisrudge luisrudge modified the milestone: v11.12.0 Oct 23, 2018
@luisrudge luisrudge changed the title [WIP] Adding default signup label Add showTerms option Nov 9, 2018
@cocojoe
Copy link
Member

cocojoe commented Nov 9, 2018

@luisrudge what happens if showterms is false but mustAcceptTerms is true?

@luisrudge
Copy link
Contributor Author

luisrudge commented Nov 9, 2018 via email

@cocojoe
Copy link
Member

cocojoe commented Nov 9, 2018

OK that seems fair as the user has explicitly disabled them.

cocojoe
cocojoe approved these changes Nov 9, 2018
View reviewed changes
Copy link
Member

@cocojoe cocojoe left a comment

Choose a reason for hiding this comment

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

LGTM

@luisrudge luisrudge merged commit 41b922a into master Nov 9, 2018
@luisrudge luisrudge deleted the feature/default-tos-string branch November 9, 2018 18:30
@luisrudge luisrudge added this to the v11.12.0 milestone Nov 9, 2018
luisrudge added a commit that referenced this pull request Nov 13, 2018
@luisrudge
Revert "Add showTerms option (#1485)"
ef75079 
This reverts commit 41b922a.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cocojoe cocojoe cocojoe approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v11.12.0
Development

Successfully merging this pull request may close these issues.
2 participants
@luisrudge @cocojoe