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

Rename the Auto Login setting to ULP; move to features tab #551

Merged
merged 3 commits into from
Oct 4, 2018

Conversation

joshcanhelp
Copy link
Contributor

Changes

There has been some general confusion around how to use the ULP with this plugin. This PR moves the setting to be more visible and changes the name to more clearly explain what the setting does. I left a reference from the old setting to the new one to help with UX.

References

Universal Login vs Embedded

Testing

No functionality changes so no testing needed.

@joshcanhelp joshcanhelp added this to the v3-Next milestone Oct 2, 2018
@joshcanhelp joshcanhelp force-pushed the change-auto-login-to-ulp branch from c262524 to 9b121aa Compare October 2, 2018 22:33
public function render_auto_login( $args = array() ) {
$this->render_switch( $args['label_for'], $args['opt_name'], 'wpa0_auto_login_method' );
$this->render_field_description(
__( 'Use the Universal Login Page (ULP) for authentication. ', 'wp-auth0' ) .
Copy link
Contributor

Choose a reason for hiding this comment

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

this is the only place you use (ULP) (in this PR at least). not a deal breaker for me, your call.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@luisrudge - Used on line 225 below as well.

Copy link
Contributor

Choose a reason for hiding this comment

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

damn tunnel vision. I was looking in other code blocks 😬

$this->render_switch( $args['label_for'], $args['opt_name'], 'wpa0_auto_login_method' );
$this->render_field_description(
__( 'Use the Universal Login Page (ULP) for authentication. ', 'wp-auth0' ) .
__( 'When turned on, <code>wp-login.php</code> will be redirected to the hosted login page. ', 'wp-auth0' ) .
Copy link
Contributor

Choose a reason for hiding this comment

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

will redirect instead of will be redirected?

luisrudge
luisrudge previously approved these changes Oct 4, 2018
@auth0 auth0 deleted a comment from codecov-io Oct 4, 2018
@codecov-io
Copy link

codecov-io commented Oct 4, 2018

Codecov Report

Merging #551 into master will decrease coverage by 0.04%.
The diff coverage is 0%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #551      +/-   ##
============================================
- Coverage     15.36%   15.32%   -0.05%     
- Complexity     1605     1606       +1     
============================================
  Files            69       69              
  Lines          5473     5489      +16     
============================================
  Hits            841      841              
- Misses         4632     4648      +16
Impacted Files Coverage Δ Complexity Δ
lib/admin/WP_Auth0_Admin_Features.php 0% <0%> (ø) 40 <2> (+2) ⬆️
lib/admin/WP_Auth0_Admin_Advanced.php 18.68% <0%> (+0.59%) 69 <1> (-1) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 607d9c2...1680863. Read the comment docs.

@joshcanhelp joshcanhelp merged commit 0e0b35a into master Oct 4, 2018
@joshcanhelp joshcanhelp deleted the change-auto-login-to-ulp branch October 4, 2018 16:53
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 19, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants