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

Change Appearance tab settings output #439

Merged
merged 3 commits into from
Apr 20, 2018

Conversation

joshcanhelp
Copy link
Contributor

  • Refactor Appearance settings with new render functions, new descriptions
  • Fix avatar Lock setting
  • Minor refactor of WP_Auth0_Admin_Features::render_password_policy() for consistency

@joshcanhelp joshcanhelp added this to the v3-Next milestone Apr 19, 2018
Copy link
Contributor

@lbalmaceda lbalmaceda left a comment

Choose a reason for hiding this comment

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

Some redaction stuff

public function render_primary_color( $args = array() ) {
$this->render_text_field( $args[ 'label_for' ], $args[ 'opt_name' ] );
$this->render_field_description(
__( 'Primary color for the Auth0 login form in hex format. ', '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.

should hex be uppercase? (I don't know either)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

__( 'Primary color for the Auth0 login form in hex format. ', 'wp-auth0' ) .
$this->get_docs_link(
'libraries/lock/v11/configuration#primarycolor-string-',
__( 'More information this settings', '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.

More information this settings
????

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's like you read these things over and over and still ...

__( 'The languageDictionary parameter for the Auth0 login form. ', 'wp-auth0' ) .
sprintf(
'<a href="https://github.com/auth0/lock/blob/master/src/i18n/en.js" target="_blank">%s</a>',
__( 'List of all modifiable text', '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.

List of all modifiable text

text ... options? or something that describes that this are items that change how the form is displayed.

$this->render_radio_button( $id_attr . '_em', $opt_name, 'email', '', 'email' === $value );
$this->render_radio_button( $id_attr . '_un', $opt_name, 'username', '', 'username' === $value );
$this->render_field_description(
__( 'To allow the user to use either email or username to login, leave this as "Auto." ', '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.

Because by reading this I get that "auto" would mean either email or username, but if the db connection has not "requires username = true" then this would only accept email.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@lbalmaceda - I was a little confused because the "requires username" setting on the DB connection was taking a while to populate on the Lock side but I see now that Lock does do the right thing even when this is wrong. I'll re-word.

public function render_gravatar( $args = array() ) {
$this->render_switch( $args[ 'label_for' ], $args[ 'opt_name' ] );
$this->render_field_description(
__( 'Automatically display avatar on login form when email address is entered', '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.

explain this are obtained from gravatar

@joshcanhelp joshcanhelp force-pushed the change-settings-fields-display-appearance branch from 9e0844c to 4f774f8 Compare April 19, 2018 22:43
Copy link
Contributor

@lbalmaceda lbalmaceda left a comment

Choose a reason for hiding this comment

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

LGTM

@joshcanhelp joshcanhelp merged commit 2dbf211 into dev Apr 20, 2018
@joshcanhelp joshcanhelp deleted the change-settings-fields-display-appearance branch April 20, 2018 16:45
@joshcanhelp joshcanhelp mentioned this pull request Jun 5, 2018
@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.

2 participants