Move masthead banner settings to Template Settings sidebar

[RachelRVasquez]

All Submissions:

• Have you followed the Contributing guidelines?
• Does your code follow the WordPress' coding standards?
• Have you checked to ensure there aren't other open Pull Requests for the same update/change?

Changes proposed in this Pull Request:

Note: We are supporting the Classic Editor plugin by detecting whether the block editor is active. When the Classic Editor plugin is on, our "Template Settings" in the sidebar are not there so the metabox settings need to remain for those users.

• Removed the checkboxes to show/hide the masthead and breadcrumbs from the metabox only if the block editor is active. (These settings only exist on pages)
• Move those options to the "Page" tab, under our "Template Settings". They use the same post meta keys as before.
• Add a new setting to allow changing the text alignment for the masthead content e.g. breadcrumbs, icon, page title/description (if they exist). (Block editor only enhancement)
• Update the existing memberlite_body_classes and memberlite_should_masthead_render functions to take both the legacy values and the new strict true/false state into account. Conditionals updated in both functions.
• This only affects pages. Archives and posts are untouched.
• Mobile tweaks have been made. Minimal re-factoring in page_banners.php so we can easily change the masthead's content alignment (left/center).
• Officially removed any remnants/code related to the "banner right" setting that was removed last year. This is now fully deprecated.
• Moved masthead styles that was in _shortcodes.scss to _masthead.scss and _pages.scss instead.

ℹ️ Note: Build files are intentionally excluded from this PR.

How to test the changes in this Pull Request:

• Test these settings on every page template: Default, Content/Sidebar, Fluid Width, Full Width, Narrow Width, Sidebar/Content. You can test Landing Page template as well, but we're deprecating this.
• Confirm mastheads show by default on existing and new pages.
• Confirm that posts and archives are unaffected.
• Confirm mastheads look good with or without a banner image.
• Confirm that the old checkboxes to turn the banner on/off and to hide breadcrumbs are no longer in the "Memberlite Settings" metabox when using WordPress without the Classic Editor plugin. These checkboxes will still be there if that plugin is on.
• Confirm that you can see the new settings under Page > Template Settings: Show Masthead Banner, Hide Breadcrumbs (Only shows when masthead is showing), and Masthead Text Alignment (Only shows when masthead is showing). - Block editor enhancement only
• Confirm that breadcrumb setting on pages doesn't show if the breadcrumb checkbox under Appearance > Customize > Breadcrumbs > Breadcrumbs on Pages is not checked.
• When page breadcrumbs are enabled, you should be able to hide them with the new toggle on a per-page basis. - Block editor enhancement only
• Test masthead combinations of the title, description, icon, breadcrumbs, and alignments, making sure they all look good on desktop and mobile.

Other information:

• Have you added an explanation of what your changes do and why you'd like us to include them?
• Have you successfully run tests with your changes locally?

Changelog entry

Classic Editor Support: Breadcrumbs and Banner toggling settings remain in their existing metabox. Functionality and styling remain unchanged. For Gutenberg: Moved the existing metabox settings on pages that toggle the top masthead/breadcrumbs to our "Template Settings" in the sidebar. Added a new setting to control text alignment. Updated body class and masthead render logic so it works for legacy and new true/false toggle values.

[Copilot]

Pull request overview

This PR moves page-level masthead banner controls (show/hide banner, hide breadcrumbs) out of the legacy page-banner metabox into the block editor “Template Settings” sidebar, and adds a new per-page setting to center-align masthead content (including optional icon/title composition). It also reorganizes associated SCSS so masthead/landing-page overrides live in more appropriate partials.

Changes:

• Adds new editor sidebar controls and registers the related page post meta in REST (including a new _memberlite_banner_text_alignment setting).
• Refactors masthead rendering to support centered alignment and extracts icon markup generation into a helper.
• Moves masthead-related styles out of shortcodes SCSS into masthead/page SCSS partials.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
src/scss/structure/_masthead.scss	Adds masthead layout rules for new centered/row icon+title structures and relocates banner-hidden spacing rule.
src/scss/pages/_page.scss	Relocates landing-template masthead overrides into the page stylesheet.
src/scss/components/_shortcodes.scss	Removes masthead-related styling from the shortcodes component stylesheet header/section.
src/editor/custom-settings.js	Adds sidebar controls (banner toggle, breadcrumb toggle, text alignment) under “Template Settings”.
js/admin-page_banners.js	Removes legacy metabox show/hide toggle JS for banner settings.
inc/page_banners.php	Removes banner show/hide + breadcrumbs controls from metabox save flow; adds alignment-aware masthead rendering and icon helper; updates banner render decision logic.
inc/extras.php	Updates breadcrumbs rendering gate to respect _memberlite_banner_hide_breadcrumbs meta more flexibly.
inc/editor-settings.php	Registers new page post meta for banner visibility, breadcrumb visibility, and text alignment; passes global breadcrumb setting to editor script.
components/header/header-masthead.php	Applies centered alignment class to masthead wrapper based on per-page meta.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[flintfromthebasement]

PR: #338 — Move masthead banner settings to Template Settings sidebar
RachelRVasquez → starter-sites | 9 files, +210 -181
#338

---

Summary
Solid refactor — moving the show/hide-breadcrumbs/alignment controls into the block editor sidebar, restricting the new meta keys to page, and cleaning out the deprecated "banner right" feature are all done consistently. One real visual regression on the masthead icon needs a fix before merge; everything else is minor.

---

Issues

• [Major] inc/page_banners.php:370 — The left-aligned icon wrapper changed from <div class="medium-1 columns text-center"> to <div class="medium-1 columns masthead-icon">. .text-center (src/scss/base/_utilities.scss:30) applied text-align: center, which horizontally centered the icon glyph in its narrow column. .masthead-icon has no matching rule anywhere in src/scss/ — I checked the full tree at this head commit. The new align-items: center on .memberlite_elements-masthead (src/scss/structure/_masthead.scss) only affects flex cross-axis (vertical) alignment, not text-align. Net effect: pages with a banner icon and left/default text alignment will show the icon left-aligned instead of centered — a visible regression from current production. Add text-align: center to .masthead-icon in _masthead.scss.

• [Minor] inc/extras.php:263 — if ( $memberlite_banner_show === '0' || ! $memberlite_banner_show ). '0' is already falsy in PHP, so === '0' is fully subsumed by ! $memberlite_banner_show and adds nothing. Simplify to if ( ! $memberlite_banner_show ) { (also drop the stray double space before !).

• [Minor] src/editor/custom-settings.js:37-38 — showBannerValue and hideBreadcrumbsValue read straight off meta?._memberlite_banner_show / meta?._memberlite_banner_hide_breadcrumbs with no fallback, unlike every other toggle in this file (hideHeaderValue, hideFooterValue, stickyValue all use || false). Before the entity record resolves, meta can be undefined, so these two ToggleControls can briefly render checked={undefined} — React will warn about a controlled/uncontrolled switch. Match the existing convention, e.g. meta?._memberlite_banner_show ?? true.

• [Minor / worth confirming] inc/page_banners.php — The classic metabox no longer renders any control for _memberlite_banner_show, _memberlite_banner_hide_breadcrumbs, or _memberlite_banner_text_alignment, and memberlite_settings_save_meta_box_data() no longer reads their $_POST values. These are now editable only through the Gutenberg sidebar panel. If any site running this theme still edits pages with the Classic Editor plugin, there's no UI path left to change these settings. Worth confirming this is an accepted trade-off — it isn't called out in the test plan.

• [Trivial] inc/editor-settings.php:218 — Diff shows \ No newline at end of file; the file lost its trailing newline.

---

Looks Good

• memberlite_get_masthead_icon_html() switching to the [fa] shortcode also fixes a pre-existing bug: the old code treated the icon-select placeholder 'blank' as a real icon (! empty( $memberlite_page_icon ) is true for 'blank'), rendering an empty icon wrapper. $show_masthead_icon now explicitly excludes 'blank'.
• register_post_meta( 'page', ... ) plus the get_post_type() === 'page' guard in memberlite_should_masthead_render() correctly scope the new settings to pages only, and default handling is sound — since these meta keys register a default, get_post_meta() correctly returns that default (true/false) rather than an empty string when a page has never been touched, so untouched pages still show the masthead.
• The breadcrumb hide check now handles both the legacy '1' string and the new boolean true from REST saves — good backward compatibility.
• Removing the dead $show_breadcrumbs/$profile_breadcrumbs locals in memberlite_get_breadcrumbs() is correct cleanup; that variable was never used after assignment.

---

Questions

1. Is dropping the classic-editor UI for these three settings (see Minor above) intentional, or should there be a metabox fallback for non-block-editor workflows?
2. Was the left-aligned icon regression (Major finding) caught in manual QA, or did testing focus on the new centered-alignment variant?

[RachelRVasquez]

Responding to Flint's Feedback:

• [Major] inc/page_banners.php:370 — The left-aligned icon wrapper changed from <div class="medium-1 columns text-center"> to <div class="medium-1 columns masthead-icon">. .text-center (src/scss/base/_utilities.scss:30) applied text-align: center, which horizontally centered the icon glyph in its narrow column. .masthead-icon has no matching rule anywhere in src/scss/ — I checked the full tree at this head commit. The new align-items: center on .memberlite_elements-masthead (src/scss/structure/_masthead.scss) only affects flex cross-axis (vertical) alignment, not text-align. Net effect: pages with a banner icon and left/default text alignment will show the icon left-aligned instead of centered — a visible regression from current production. Add text-align: center to .masthead-icon in _masthead.scss.

I checked again and compared my branch to Memberlite 6.1 from a few versions ago, and from what I'm seeing, yes, that .text-center class was there, but with the padding on that narrow column, and with how large the icons are, it was essentially doing nothing. Even if I remove the class, it still looks the same:

• [Minor] inc/extras.php:263 — if ( $memberlite_banner_show === '0' || ! $memberlite_banner_show ). '0' is already falsy in PHP, so === '0' is fully subsumed by ! $memberlite_banner_show and adds nothing. Simplify to if ( ! $memberlite_banner_show ) { (also drop the stray double space before !).

Since '0' is a string, it's not considered falsy. Since the '1' and '0' values are from the legacy metabox, there's no differentiating the difference between '0' and an empty string if the value is never saved on a page, but the banner still needs to be "true" despite that/by default. So to ensure both legacy and the new true/false toggle from the Template Settings sidebar works properly, with the banner showing (true) if the string is empty and if it's explicitly 1 as a string, we have the additional conditional check.

• [Minor] src/editor/custom-settings.js:37-38 — showBannerValue and hideBreadcrumbsValue read straight off meta?._memberlite_banner_show / meta?._memberlite_banner_hide_breadcrumbs with no fallback, unlike every other toggle in this file (hideHeaderValue, hideFooterValue, stickyValue all use || false)...

This is intentional. Detailed explanation here: #338 (comment)

• [Minor / worth confirming] inc/page_banners.php — The classic metabox no longer renders any control for _memberlite_banner_show, _memberlite_banner_hide_breadcrumbs, or _memberlite_banner_text_alignment, and memberlite_settings_save_meta_box_data() no longer reads their $_POST values. These are now editable only through the Gutenberg sidebar panel. If any site running this theme still edits pages with the Classic Editor plugin, there's no UI path left to change these settings. Worth confirming this is an accepted trade-off — it isn't called out in the test plan.

We've already done this with other settings we've deprecated from the metabox and moved to the sidebar. So this is probably okay, but I'll confirm whether we support the Classic Editor plugin.

[flintfromthebasement]

Follow-up review

PR: #338 — Move masthead banner settings to Template Settings sidebar
RachelRVasquez → starter-sites | 9 files, +209 -180
#338

---

Summary
Delta since my last pass is just the trailing-newline restore in editor-settings.php — no functional code changed. All of my prior points have been answered by the author; two hold up, two don't fully. Nothing new or blocking. Still a COMMENT-level PR, ready to merge once the two open Minor items below are addressed (or explicitly waived).

---

Prior issues — status

• Masthead icon left-align regression (inc/page_banners.php:370, .text-center → .masthead-icon) — Pushed back, and I'll concede it. You compared against 6.1 and showed a screenshot with .text-center removed entirely looking identical — the narrow medium-1 column's padding plus the large icon glyph (size="4x" in memberlite_get_masthead_icon_html()) apparently leaves little room for text-align to matter. That's real QA evidence, not theory, so I'm downgrading this off the blocking list. One thing worth a quick check since it wasn't in the screenshot: templates/narrow-width.php renders the icon at size="2x" (inc/page_banners.php:313) — a smaller glyph in the same column has more slack, so the same regression could still show up there even if it doesn't at 4x. Worth a 30-second look before calling this fully closed.

• inc/extras.php:263 — $memberlite_banner_show === '0' || ! $memberlite_banner_show — Holding the line, but downgrading to Trivial. Checked this directly: php -r 'var_dump((bool)"0");' → false. PHP does treat the string '0' as falsy in a boolean context — ! "0" is true — so === '0' never adds a case that !$memberlite_banner_show doesn't already catch. This isn't the "string vs boolean" gap you're describing; that gap is real for other values ('' from REST, unset returning the registered default) but '0' isn't one of them since it's already falsy. No behavior changes if you simplify to if ( ! $memberlite_banner_show ) { — pure redundancy, not a bug, and I won't hold the PR on it.

• src/editor/custom-settings.js:37-38 — no fallback on showBannerValue/hideBreadcrumbsValue — Partially addressed / still open on the actual fix. Your explanation (and Copilot's independent flag on the same line) both correctly note that || false would be wrong for showBannerValue since the registered default is true (inc/editor-settings.php:87) — agreed, || can't tell "never set" apart from "explicitly off." But that's not what I suggested. ?? (nullish coalescing) checks only for null/undefined, not falsiness — so meta?._memberlite_banner_show ?? true resolves undefined (mid-load) to true without touching an explicitly-saved false/''/'0'. That's the fix that satisfies your own constraint. For hideBreadcrumbsValue the registered default is false, so it doesn't have this asymmetry at all — || false, matching the other toggles, is fine there and removes the transient checked={undefined} warning during load.

• Classic editor UI removal (inc/page_banners.php) — Acknowledged, not yet confirmed. You said you'll check whether Classic Editor plugin support is expected; no update yet. Not blocking, just don't let it fall off before merge.

• inc/editor-settings.php:218 trailing newline — Resolved. Confirmed in the diff.

---

New findings since last review
None — the only change in this range is the newline fix above.

---

Questions

1. Did you check the narrow-width template's 2x icon rendering for the same left-align comparison, or only the default 4x size?
2. Any update on Classic Editor support for these three settings?

[RachelRVasquez]

More Flint follow-up:

1. Did you check the narrow-width template's 2x icon rendering for the same left-align comparison, or only the default 4x size?

I did. The .text-center still does nothing to the icon visually with the exception of mobile, when the icon centers because there's enough room for it. On this branch, it's been adjusted so the left aligned remains left aligned (icon still on top of page title) by default, and the "center" option will center everything on mobile. Less of a regression, but more of a design choice so that our new settings are clear. Everything left aligned vs everything center aligned. Again, the icon's position change would only be obvious on mobile.

2. Any update on Classic Editor support for these three settings?

Following up on this internally.

[flintfromthebasement]

Follow-up review

PR: #338 — Move masthead banner settings to Template Settings sidebar
RachelRVasquez → starter-sites | 9 files, +209 -180
#338

---

Summary
Good progress — the left-align icon question is closed with real evidence, and Classic Editor support is now restored (my open question from last round). One issue I've now flagged three rounds running is still not fixed: the ?? fallback on showBannerValue. Nothing new is blocking; still COMMENT-level.

---

Prior issues — status

• Masthead icon left-align regression — Resolved. header-masthead.php now only adds text-center when _memberlite_banner_text_alignment === 'centered' ($masthead_classes = $is_page_masthead_centered ? 'masthead text-center' : 'masthead';), and the centered layout uses an explicit flex wrapper (.memberlite_elements-masthead-centered, _masthead.scss) instead of relying on inherited text-align. Per the thread, you also checked the narrow-width 2x icon and confirmed the mobile centering is now an intentional design choice (left stays left, center centers everything), not an accidental regression. Closing this out.

• inc/extras.php:263 redundant === '0' || !$x check — Not addressed, still Trivial. Unchanged since I downgraded it last round — not worth holding the PR on.

• src/editor/custom-settings.js — no ?? fallback on showBannerValue/hideBreadcrumbsValue — Still not addressed. Current code:

  const showBannerValue = meta?._memberlite_banner_show;
  const hideBreadcrumbsValue = meta?._memberlite_banner_hide_breadcrumbs;

  This is the third round I've flagged this. To recap the actual fix (not ||, which you correctly noted breaks the true-default case): meta?._memberlite_banner_show ?? true resolves undefined (before the entity record loads) to true without touching an explicitly-saved false. hideBreadcrumbsValue has no such asymmetry (default is false), so meta?._memberlite_banner_hide_breadcrumbs || false is fine there and matches the other toggles in the file. As-is, both ToggleControls render checked={undefined} on first paint, which can visibly flash the "Show Masthead Banner" toggle off before flipping on, plus a React controlled/uncontrolled console warning. Low severity but trivial to fix and now overdue.

• Classic editor UI removal — Resolved. page_banners.php now gates the metabox fields with the new memberlite_is_block_editor() helper — classic editor still gets the radio/checkboxes, block editor gets a pointer to the sidebar panel instead. js/admin-page_banners.js also correctly no-ops when the radio doesn't exist. Good fix.

---

New findings

• [Minor] functions.php — memberlite_is_block_editor():

  function memberlite_is_block_editor() {
      $current_screen = get_current_screen();
      return method_exists( $current_screen, 'is_block_editor' ) && $current_screen->is_block_editor();
  }

  get_current_screen() returns null outside a set admin-screen context (e.g. a REST-driven save_post from the block editor's own autosave, or a CLI/cron-triggered save). method_exists( null, 'is_block_editor' ) doesn't fatal, but passing null to a non-nullable internal parameter throws a deprecation notice on PHP 8.1+. Guard with $current_screen instanceof WP_Screen && $current_screen->is_block_editor() instead. Low risk since the isset($_POST[...]) guards in memberlite_settings_save_meta_box_data() prevent any actual bad write, but it'll spam the PHP log on REST saves.

• Unrelated changes in this diff — functions.php (memberlite_dedupe_editor_color_palette → memberlite_clean_editor_color_palette, now curating against a hardcoded allowlist of ~15 slugs instead of deduping by color value), inc/colors.php (hex value changes for Deep Harbor/Cocoa Ash/Gotham/Rosewood), and structure/_header.scss (removal of the .scheme_cocoa_ash color-mix workaround, consistent with the Cocoa Ash primary color now being lighter) all showed up in this diff range but have nothing to do with masthead banner settings. This looks like a rebase pulling in commits from an updated starter-sites base rather than something you added to this PR — worth confirming so it's not accidentally double-reviewed or double-merged elsewhere.

---

Questions

1. Can you confirm the color-palette/hex changes (functions.php, inc/colors.php, _header.scss) landed here via a rebase off starter-sites, not as new work on this branch?

[flintfromthebasement]

Follow-up review

PR: #338 — Move masthead banner settings to Template Settings sidebar
RachelRVasquez → starter-sites | 10 files, +272 -155
#338

---

Summary
Both outstanding items from my last pass are now fixed correctly. Nothing new is wrong. Approving.

---

Prior issues — status

• functions.php — memberlite_is_block_editor() null-screen deprecation notice — Resolved. Now return $current_screen instanceof WP_Screen && $current_screen->is_block_editor();. instanceof on null is safe (returns false, no notice), so this correctly avoids the PHP 8.1 deprecation I flagged from passing null into method_exists()'s second-argument-adjacent call pattern.

• src/editor/custom-settings.js — no ?? fallback on showBannerValue (flagged 3 rounds running) — Resolved. Now meta?._memberlite_banner_show ?? true with an inline comment explaining the ?? vs || distinction, and hideBreadcrumbsValue uses || false (correct, since its default is false and has no asymmetry). This is exactly the fix I'd been asking for — verified against the diff, no checked={undefined} case remains on first paint.

• Unrelated color-palette/hex changes (functions.php, inc/colors.php, _header.scss) I flagged as a possible rebase artifact — Withdrawing, my error. I re-diffed this PR branch against a freshly-fetched upstream/starter-sites merge-base (5c3bd50) and neither inc/colors.php nor structure/_header.scss appear in the diff at all — the current 10-file diff is functions.php, inc/editor-settings.php, inc/extras.php, inc/page_banners.php, js/admin-page_banners.js, src/editor/custom-settings.js, components/header/header-masthead.php, and the three _shortcodes.scss/_page.scss/_masthead.scss files (the masthead SCSS reorg, which is legitimately part of this PR's scope). The color-palette diff I reported last round was against a stale local starter-sites checkout on my end that hadn't picked up those already-merged upstream commits — not something in your branch. Sorry for the noise; no action needed on your side.

• inc/extras.php:263 redundant === '0' || !$x — Still open, still Trivial. Unchanged, not worth holding the PR on.

---

New findings
None — the delta since my last review is exactly the two fixes above.