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

Fix widget gravatar and language settings #706

Merged
merged 1 commit into from
Aug 1, 2019

Conversation

joshcanhelp
Copy link
Contributor

Changes

  • Fix admin gravatar label not attached to correct input
  • Fix gravatar override setting not working
  • Fix translation setting not working

Note to reviewers: PO, MO, and POT files can be ignored.

@joshcanhelp joshcanhelp added this to the 3.11.1 milestone Jul 31, 2019
@joshcanhelp joshcanhelp requested a review from a team July 31, 2019 21:03
@@ -113,9 +118,14 @@ protected function build_settings( $settings ) {

$options_obj['socialButtonStyle'] = 'big';

if ( isset( $settings['gravatar'] ) && empty( $settings['gravatar'] ) ) {
if ( isset( $settings['gravatar'] ) && '' !== $settings['gravatar'] && empty( $settings['gravatar'] ) ) {
Copy link
Contributor

Choose a reason for hiding this comment

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

empty is already "". If you check for !="" and empty at the same time, this will never be called! What is going on here? 🤕

'' !== $settings['gravatar'] && empty( $settings['gravatar'] )

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 a little wonky because it has a number of settings arrays that override each other with successive option building.

'' is an empty string (in this case, means use the default), isset() is whether that key exists or not (avoids Undefined index errors), and empty() checks for 0, null, false, etc.

$options_obj['avatar'] = null;
}

if ( ! empty( $settings['gravatar'] ) ) {
Copy link
Contributor

Choose a reason for hiding this comment

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

you check for empty but not for isset, why ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

empty() covers the isset() case. Means "no truth-y value to use".

templates/a0-widget-setup-form.php Show resolved Hide resolved
@joshcanhelp joshcanhelp merged commit e220145 into wordpress-org-plugin Aug 1, 2019
@joshcanhelp joshcanhelp deleted the fix-widget-admin-form branch August 1, 2019 18:21
@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.

None yet

2 participants