Skip to content

Site Logo: show the current logo in WP Admin General Settings#50764

Draft
taipeicoder wants to merge 19 commits into
trunkfrom
edi-639-show-the-current-site-logo-in-wp-admin
Draft

Site Logo: show the current logo in WP Admin General Settings#50764
taipeicoder wants to merge 19 commits into
trunkfrom
edi-639-show-the-current-site-logo-in-wp-admin

Conversation

@taipeicoder

Copy link
Copy Markdown
Contributor

Proposed changes

  • The Site Logo field on the WP Admin General Settings page (WordPress.com) previously rendered only the Fiverr logo-maker upsell and never read the site's stored logo, so the field always appeared empty — even when a logo had been set from the Site Editor's Identity section. This made it look like no logo existed.
  • The field now reads the current logo (the site_logo option, falling back to the custom_logo theme mod, which core keeps in sync) and shows a preview when one is set. The Fiverr CTA becomes the empty state, shown only when no logo exists; the "Try Fiverr Logo Maker" button remains available in both states.

Related product discussion/links

  • EDI-639

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

No.

Testing instructions

  • On a WordPress.com Simple or Atomic site, go to Settings → General in WP Admin.
  • With no Site Logo set, confirm the "Site Logo" row shows the Fiverr upsell ("Make an incredible logo in minutes" + "Try Fiverr Logo Maker") as before.
  • Set a Site Logo via the Site Editor's Design → Styles → Identity section (or Appearance → Editor), then reload Settings → General.
  • Confirm the "Site Logo" row now shows a preview of the current logo, with the Fiverr button still available.

The "Site Logo" field on the General Settings page rendered only the Fiverr
upsell and never read the stored logo, so it always looked empty even when a
logo had been set from the Site Editor's Identity section.

Read the current logo (site_logo option, falling back to the custom_logo theme
mod) and render a preview when one is set, keeping the Fiverr CTA as the empty
state.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@github-actions

github-actions Bot commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

Are you an Automattician? Please test your changes on all WordPress.com environments to help mitigate accidental explosions.

  • To test on WoA, go to the Plugins menu on a WoA dev site. Click on the "Upload" button and follow the upgrade flow to be able to upload, install, and activate the Jetpack Beta plugin. Once the plugin is active, go to Jetpack > Jetpack Beta, select your plugin (WordPress.com Site Helper), and enable the edi-639-show-the-current-site-logo-in-wp-admin branch.
  • To test on Simple, run the following command on your sandbox:
bin/jetpack-downloader test jetpack-mu-wpcom-plugin edi-639-show-the-current-site-logo-in-wp-admin

Interested in more tips and information?

  • In your local development environment, use the jetpack rsync command to sync your changes to a WoA dev blog.
  • Read more about our development workflow here: PCYsg-eg0-p2
  • Figure out when your changes will be shipped to customers here: PCYsg-eg5-p2

@github-actions

github-actions Bot commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

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.
  • ✅ Add a "[Status]" label (In Progress, Needs 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 🤖


Follow this PR Review Process:

  1. Ensure all required checks appearing at the bottom of this PR are passing.
  2. Make sure to test your changes on all platforms that it applies to. You're responsible for the quality of the code you ship.
  3. You can use GitHub's Reviewers functionality to request a review.
  4. When it's reviewed and merged, you will be pinged in Slack to deploy the changes to WordPress.com simple once the build is done.

If you have questions about anything, reach out in #jetpack-developers for guidance!

@github-actions github-actions Bot added the [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. label Jul 23, 2026
@jp-launch-control

jp-launch-control Bot commented Jul 23, 2026

Copy link
Copy Markdown

Code Coverage Summary

Coverage changed in 1 file.

File Coverage Δ% Δ Uncovered
projects/packages/jetpack-mu-wpcom/src/features/wpcom-options-general/options-general.php 0/123 (0.00%) 0.00% 75 💔

Full summary · PHP report · JS report

Coverage check overridden by I don't care about code coverage for this PR Use this label to ignore the check for insufficient code coveage. .

taipeicoder and others added 6 commits July 23, 2026 11:38
Address review feedback on the WP Admin General Settings logo preview:

- Request a proportional image size instead of a square, so wide wordmarks are
  no longer shown as a center-cropped fragment; cap the dimensions in CSS.
- Give the image an explicit alt, falling back to the site title when the
  attachment has none, so the populated state is announced to screen readers.
- Normalize the site_logo option (attachment ID, legacy array, or WP_Error)
  before casting, avoiding a warning and a bogus attachment ID.
- Render the preview on a checkerboard surface so white, dark, and transparent
  logos all stay visible; use logical CSS properties.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The custom_logo theme-mod fallback was cast to int without the same guard used
for the site_logo option. Core's site-logo filter can surface a WP_Error through
either source, so casting the fallback directly could emit a warning and produce
attachment ID 1. Route both candidate values through a shared normalization
helper that only yields a positive scalar ID.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
wp_get_attachment_image() emits explicit width/height attributes, so clamping
only one axis let the two max constraints distort non-square logos. Set both
inline-size and block-size to auto so the image scales proportionally within the
bounds. Make the preview container block-level but shrink-wrapped so the Fiverr
button stays below it, matching the empty state across viewport widths.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The previous tile contrast (#fff vs #f0f0f1) was too subtle to reveal white or
near-white transparent logos, leaving the row looking blank. Use medium-gray
tiles over a light base so both white and dark logos remain legible.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Collapse the four-layer linear-gradient checkerboard into a single
conic-gradient, dropping the hand-tuned background-position offsets. Drop the
redundant display: block on the div, use consistent truthy id guards, and trim
the normalization docblock.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@taipeicoder taipeicoder added the I don't care about code coverage for this PR Use this label to ignore the check for insufficient code coveage. label Jul 23, 2026
taipeicoder and others added 2 commits July 23, 2026 13:02
The single conic-gradient can't satisfy stylelint here: it exceeds the 80-column
limit on one line, but the config also forbids breaking a function's arguments
across lines. Restore the four-layer linear-gradient form, which lints cleanly.
The other simplifications (dropped redundant display, consistent guards) stand.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Show the "Make an incredible logo in minutes" promo text in both states rather
than only the empty state, so it reads as persistent helper copy under the field
(like the site-address domain description) with the logo preview above it when
one is set.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@taipeicoder taipeicoder self-assigned this Jul 23, 2026
taipeicoder and others added 10 commits July 23, 2026 13:46
Move the promo text beneath the "Try Fiverr Logo Maker" button and mark it up as
a p.description, matching the site-address helper text, so the button leads and
the copy reads as supporting description.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…reen

Replace the promo copy with text explaining that the logo displays via the Site
Logo block, and link "You can change your site logo in the site editor" to the
Site Editor identity screen.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Lay the field out as a wrapping flex row so the logo preview and the Fiverr
button share the first line and the description spans a full-width line beneath
them. Size the preview to sit compactly next to the button, and end the
description sentence with a period.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Only apply the new preview and description to the logo-present state; when no
logo is set, keep the original Fiverr promo copy and button unchanged. Extract
the shared button markup into a helper so both states reuse it.

Link to the Site Editor's Identity screen only on block themes, where it exists;
classic themes have no Site Editor, so point them at the Customizer's logo
control instead.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Resolve the logo edit destination through a helper: on block themes, deep-link to
the Site Editor's Identity screen only when the route is available (Gutenberg 22.8
/ WordPress 7.1), otherwise fall back to the default Site Editor. On classic
themes, target the Customizer control that matches the theme's logo integration
(custom_logo or site_logo), matching the logo-tool feature.

Let the preview row wrap so the button drops below the preview on narrow screens
instead of overflowing, and cap the preview at the container width.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
A classic theme without custom-logo or site-logo support can still show a preview
from a leftover site_logo option, but its Customizer has no logo control. Return
an empty URL in that case and skip the description so the UI does not promise an
edit path that leads nowhere.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Dropping to the generic Site Editor when the Identity screen is unavailable could
be a dead end (no logo control if the active templates lack a Site Logo block),
and the accompanying copy would overclaim. Return an empty URL in that case so the
description is omitted, and only show the change link where Identity is available.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Fall back to "Current site logo" when the site title is empty, instead of
"Current site logo for " with a dangling preposition.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Trim the verbose explanatory comments in the options-general PHP and SCSS to
concise one- or two-liners; no behavior change.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Use 4px preview padding and a 30px image cap so the preview totals 40px
(30 + 4px padding + 1px border each side), aligning it with the button, and
tighten the row gap to 10px.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

I don't care about code coverage for this PR Use this label to ignore the check for insufficient code coveage. [mu wpcom Feature] Wpcom Options General [Package] Jetpack mu wpcom WordPress.com Features [Status] In Progress [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant