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

Jetpack Site Logo: add integer check for custom_logo #20049

Merged
merged 3 commits into from
Jun 9, 2021

Conversation

fullofcaffeine
Copy link
Contributor

@fullofcaffeine fullofcaffeine commented Jun 8, 2021

Summary:
Resolves 53517-gh-wp-calypso and supports #19654

Implementation by @creativecoder.

Code recently added to Gutenberg syncs site_logo options and custom_logo theme mod. This causes a conflict with the legacy Jetpack site logo functionality.

This is an initial fix--we'll need to follow up to reconcile the new site_logo option and Jetpacks legacy logo functionality.

Related convo: p1623174912090300-slack-CBTN58FTJ.

Test Plan:

  • Sandbox a Simple Site
  • Activate a theme that uses jetpack_the_site_logo to display a logo, such as premium/soho (wp theme activate premium/soho --url=[private link])
  • Navigate to the customizer, set a logo, and save the changes
  • Without this change, the logo does not display
  • With this change, the logo should display

Reviewers: paulbunkham, kraftbj, fullofcaffeine, bernhardreiter

Tags: #touches_jetpack_files

Differential Revision: D62504-code

This commit syncs r226957-wpcom.

Fixes #

Changes proposed in this Pull Request:

Jetpack product discussion

Does this pull request change what data or activity we track or use?

Testing instructions:

  • Go to '..'

Summary:
Resolves 53517-gh-wp-calypso

Code recently added to Gutenberg syncs site_logo options and custom_logo theme mod. This causes a conflict with the legacy Jetpack site logo functionality.

This is an initial fix--we'll need to follow up to reconcile the new site_logo option and Jetpacks legacy logo functionality.

Related convo: p1623174912090300-slack-CBTN58FTJ.

Test Plan:
- Sandbox a Simple Site
- Activate a theme that uses `jetpack_the_site_logo` to display a logo, such as premium/soho (`wp theme activate premium/soho --url=[private link]`)
- Navigate to the customizer, set a logo, and save the changes
- Without this change, the logo does not display
- With this change, the logo should display

Reviewers: paulbunkham, kraftbj, fullofcaffeine, bernhardreiter

Tags: #touches_jetpack_files

Differential Revision: D62504-code

This commit syncs r226957-wpcom.
@kraftbj kraftbj added this to the jetpack/9.8.2 milestone Jun 8, 2021
@kraftbj kraftbj self-requested a review June 8, 2021 21:46
@kraftbj kraftbj added [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. [Type] Dotcom Merge [Feature] Theme Tools labels Jun 8, 2021
@github-actions github-actions bot added the [Plugin] Jetpack Issues about the Jetpack plugin. https://wordpress.org/plugins/jetpack/ label Jun 8, 2021
@github-actions
Copy link
Contributor

github-actions bot commented Jun 8, 2021

Thank you for your PR!

When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:

  • ✅ Include a description of your PR changes.
  • ✅ All commits were linted before commit.
  • ✅ Add a "[Status]" label (In Progress, Needs Team Review, ...).
  • ✅ Add testing instructions.
  • ✅ Specify whether this PR includes any changes to data or privacy.
  • ✅ Add changelog entries to affected projects

This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖


Once your PR is ready for review, check one last time that all required checks (other than "Required review") appearing at the bottom of this PR are passing or skipped.
Then, add the "[Status] Needs Team review" label and ask someone from your team review the code.
Once you’ve done so, switch to the "[Status] Needs Review" label; someone from Jetpack Crew will then review this PR and merge it to be included in the next Jetpack release.


Jetpack plugin:

  • Next scheduled release: July 6, 2021.
  • Scheduled code freeze: June 28, 2021.

@github-actions github-actions bot added [Status] Needs Author Reply We would need you to make some changes or provide some more details about your PR. Thank you! and removed [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. labels Jun 8, 2021
@kraftbj kraftbj removed the [Status] Needs Author Reply We would need you to make some changes or provide some more details about your PR. Thank you! label Jun 8, 2021
@kraftbj kraftbj enabled auto-merge (squash) June 8, 2021 21:49
@github-actions github-actions bot added the [Status] Needs Author Reply We would need you to make some changes or provide some more details about your PR. Thank you! label Jun 8, 2021
@kraftbj kraftbj added [Status] Ready to Merge Go ahead, you can push that green button! and removed [Status] Needs Author Reply We would need you to make some changes or provide some more details about your PR. Thank you! labels Jun 9, 2021
@kraftbj kraftbj merged commit 3f2a37c into master Jun 9, 2021
@kraftbj kraftbj deleted the fusion-sync/fullofcaffeine/r226957-wpcom-1623188698 branch June 9, 2021 02:35
@github-actions github-actions bot removed the [Status] Ready to Merge Go ahead, you can push that green button! label Jun 9, 2021
@jeherve jeherve modified the milestones: jetpack/9.8.2, jetpack/9.9 Jun 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Theme Tools [Plugin] Jetpack Issues about the Jetpack plugin. https://wordpress.org/plugins/jetpack/ Touches WP.com Files [Type] Dotcom Merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants