Show breadcrumbs in individual theme header #94038

allilevine · 2024-08-29T20:15:37Z

Proposed Changes

Use the NavigationHeader component for individual themes.
Update the Breadcrumb and NavigationHeader components to take an onClick prop.

Before	After

Why are these changes being made?

To keep the page header consistent with other pages, especially the theme showcase and individual plugin pages.

Testing Instructions

Go to /themes.
Click on a theme.
Verify that the breadcrumb text is correct (Themes > title of the theme).
Use the header to navigate back to Themes.
Repeat at site level (Appearance > Themes).
Repeat while logged out.
Test at different screen sizes.
Regression test another page with breadcrumbs, like Stats and individual Plugins pages.

Known issue: there's some variation in padding on the page. I'm aligning with the theme title.

Pre-merge Checklist

Has the general commit checklist been followed? (PCYsg-hS-p2)
Have you written new tests for your changes?
Have you tested the feature in Simple (P9HQHe-k8-p2), Atomic (P9HQHe-jW-p2), and self-hosted Jetpack sites (PCYsg-g6b-p2)?
Have you checked for TypeScript, React or other console errors?
Have you used memoizing on expensive computations? More info in Memoizing with create-selector and Using memoizing selectors and Our Approach to Data
Have we added the "[Status] String Freeze" label as soon as any new strings were ready for translation (p4TIVU-5Jq-p2)?
For changes affecting Jetpack: Have we added the "[Status] Needs Privacy Updates" label if this pull request changes what data or activity we track or use (p4TIVU-aUh-p2)?

github-actions · 2024-08-29T20:15:49Z

Calypso Live (direct link)

	https://calypso.live?image=registry.a8c.com/calypso/app:build-118696

Jetpack Cloud live (direct link)

	https://calypso.live?image=registry.a8c.com/calypso/app:build-118696&env=jetpack

Automattic for Agencies live (direct link)

	https://calypso.live?image=registry.a8c.com/calypso/app:build-118696&env=a8c-for-agencies

matticbot · 2024-08-29T20:20:21Z

This PR modifies the release build for the following Calypso Apps:

For info about this notification, see here: PCYsg-OT6-p2

notifications
odyssey-stats
wpcom-block-editor

To test WordPress.com changes, run install-plugin.sh $pluginSlug add/theme-header-breadcrumbs on your sandbox.

matticbot · 2024-08-29T20:24:18Z

Here is how your PR affects size of JS and CSS bundles shipped to the user's browser:

App Entrypoints (~619 bytes removed 📉 [gzipped])

name                   parsed_size           gzip_size
entry-stepper              -1269 B  (-0.1%)     -479 B  (-0.1%)
entry-main                 -1269 B  (-0.1%)     -499 B  (-0.1%)
entry-subscriptions         -580 B  (-0.0%)     -195 B  (-0.0%)
entry-login                 -580 B  (-0.0%)     -277 B  (-0.1%)
entry-domains-landing       -580 B  (-0.1%)     -277 B  (-0.1%)
entry-browsehappy           -580 B  (-0.3%)     -277 B  (-0.4%)

Common code that is always downloaded and parsed every time the app is loaded, no matter which route is used.

Sections (~25884 bytes added 📈 [gzipped])

name                                parsed_size           gzip_size
theme                                  +16548 B  (+2.3%)    +3739 B  (+1.7%)
link-in-bio-tld-flow                     -798 B  (-0.0%)      -67 B  (-0.0%)
update-design-flow                       -760 B  (-0.1%)      -34 B  (-0.0%)
signup                                   -760 B  (-0.3%)      -56 B  (-0.1%)
themes                                   -387 B  (-0.0%)     -494 B  (-0.2%)
plugins                                  -216 B  (-0.0%)     -970 B  (-0.1%)
jetpack-cloud-plugin-management          -216 B  (-0.0%)     -970 B  (-0.2%)
checkout                                 -215 B  (-0.0%)    -1164 B  (-0.2%)
readymade-template-flow                  +196 B  (+0.1%)    +1040 B  (+3.4%)
home                                     -178 B  (-0.0%)      -91 B  (-0.0%)
domains                                  +176 B  (+0.0%)     +404 B  (+0.1%)
devdocs                                  -176 B  (-0.1%)      -39 B  (-0.1%)
plans                                    +160 B  (+0.0%)     +416 B  (+0.1%)
marketplace                              -101 B  (-0.0%)     -853 B  (-0.3%)
write-flow                                -38 B  (-0.0%)      -11 B  (-0.0%)
videopress-flow                           -38 B  (-0.0%)      -11 B  (-0.0%)
build-flow                                -38 B  (-0.0%)      -11 B  (-0.0%)
staging-site                              -20 B  (-0.0%)       -6 B  (-0.0%)
settings-security                         -20 B  (-0.0%)       -1 B  (-0.0%)
settings-podcast                          -20 B  (-0.0%)       -6 B  (-0.0%)
settings-performance                      -20 B  (-0.0%)       +0 B
settings                                  -20 B  (-0.0%)       -6 B  (-0.0%)
posts-custom                              -20 B  (-0.0%)     -186 B  (-0.1%)
posts                                     -20 B  (-0.0%)     -186 B  (-0.1%)
media                                     -20 B  (-0.0%)       -5 B  (-0.0%)
marketing                                 -20 B  (-0.0%)       +0 B
hosting                                   -20 B  (-0.0%)      -65 B  (-0.0%)
earn                                      -20 B  (-0.0%)       -8 B  (-0.0%)
activity                                  -20 B  (-0.0%)       -1 B  (-0.0%)
woocommerce-installation                  +18 B  (+0.0%)      +10 B  (+0.0%)
subscribers                               +18 B  (+0.0%)       +4 B  (+0.0%)
stats                                     +18 B  (+0.0%)       +4 B  (+0.0%)
sites-dashboard                           +18 B  (+0.0%)       +6 B  (+0.0%)
site-purchases                            +18 B  (+0.0%)       +4 B  (+0.0%)
site-monitoring                           +18 B  (+0.0%)       +6 B  (+0.0%)
site-logs                                 +18 B  (+0.0%)       +6 B  (+0.0%)
site-blocks                               +18 B  (+0.0%)      +10 B  (+0.0%)
settings-writing                          +18 B  (+0.0%)      +10 B  (+0.0%)
settings-reading                          +18 B  (+0.0%)      +10 B  (+0.0%)
settings-newsletter                       +18 B  (+0.0%)      +10 B  (+0.0%)
settings-jetpack                          +18 B  (+0.0%)      +10 B  (+0.0%)
settings-discussion                       +18 B  (+0.0%)      +10 B  (+0.0%)
security                                  +18 B  (+0.0%)       +8 B  (+0.0%)
scan                                      +18 B  (+0.0%)      +10 B  (+0.0%)
reader                                    +18 B  (+0.0%)      +39 B  (+0.0%)
purchases                                 +18 B  (+0.0%)       +4 B  (+0.0%)
privacy                                   +18 B  (+0.0%)      +10 B  (+0.0%)
people                                    +18 B  (+0.0%)      +10 B  (+0.0%)
pages                                     +18 B  (+0.0%)      +10 B  (+0.0%)
notification-settings                     +18 B  (+0.0%)      +10 B  (+0.0%)
me                                        +18 B  (+0.0%)      +10 B  (+0.0%)
jetpack-cloud-settings                    +18 B  (+0.0%)      +10 B  (+0.0%)
jetpack-cloud-partner-portal              +18 B  (+0.0%)       +6 B  (+0.0%)
jetpack-cloud-agency-sites-v2             +18 B  (+0.0%)       +6 B  (+0.0%)
import                                    +18 B  (+0.0%)      +10 B  (+0.0%)
hosting-features                          +18 B  (+0.0%)       +6 B  (+0.0%)
help                                      +18 B  (+0.0%)      +10 B  (+0.0%)
google-my-business                        +18 B  (+0.0%)      +10 B  (+0.0%)
github-deployments                        +18 B  (+0.0%)       +6 B  (+0.0%)
export                                    +18 B  (+0.0%)      +10 B  (+0.0%)
email                                     +18 B  (+0.0%)      +66 B  (+0.0%)
developer                                 +18 B  (+0.0%)      +10 B  (+0.0%)
comments                                  +18 B  (+0.0%)      +10 B  (+0.0%)
backup                                    +18 B  (+0.0%)       +6 B  (+0.0%)
add-ons                                   +18 B  (+0.0%)      +10 B  (+0.0%)
account-close                             +18 B  (+0.0%)      +10 B  (+0.0%)
account                                   +18 B  (+0.0%)       +9 B  (+0.0%)
a8c-for-agencies-team                     +18 B  (+0.0%)       +6 B  (+0.0%)
a8c-for-agencies-sites                    +18 B  (+0.0%)       +6 B  (+0.0%)
a8c-for-agencies-settings                 +18 B  (+0.0%)      +10 B  (+0.0%)
a8c-for-agencies-referrals                +18 B  (+0.0%)       +6 B  (+0.0%)
a8c-for-agencies-purchases                +18 B  (+0.0%)      +10 B  (+0.0%)
a8c-for-agencies-plugins                  +18 B  (+0.0%)      +10 B  (+0.0%)
a8c-for-agencies-partner-directory        +18 B  (+0.0%)      +10 B  (+0.0%)
a8c-for-agencies-overview                 +18 B  (+0.0%)       +6 B  (+0.0%)
a8c-for-agencies-migrations               +18 B  (+0.0%)      +10 B  (+0.0%)
a8c-for-agencies-marketplace              +18 B  (+0.0%)      +10 B  (+0.0%)
a8c-for-agencies-landing                  +18 B  (+0.0%)      +10 B  (+0.0%)
a8c-for-agencies-client                   +18 B  (+0.0%)       +6 B  (+0.0%)

Sections contain code specific for a given set of routes. Is downloaded and parsed only when a particular route is navigated to.

Async-loaded Components (~963 bytes removed 📉 [gzipped])

name                                                                              parsed_size           gzip_size
async-load-design-blocks                                                               -302 B  (-0.0%)     -952 B  (-0.1%)
async-load-signup-steps-theme-selection                                                -264 B  (-0.1%)     -788 B  (-0.7%)
async-load-signup-steps-design-picker                                                  -208 B  (-0.4%)      -36 B  (-0.2%)
async-load-automattic-help-center                                                      -196 B  (-0.0%)     -863 B  (-0.5%)
async-load-calypso-reader-sidebar                                                       -38 B  (-0.0%)      -11 B  (-0.0%)
async-load-calypso-post-editor-editor-media-modal                                       -38 B  (-0.0%)      -14 B  (-0.0%)
async-load-calypso-my-sites-stats-summary                                               -38 B  (-0.1%)       -9 B  (-0.1%)
async-load-calypso-my-sites-current-site-notice                                         -38 B  (-0.1%)      -13 B  (-0.1%)
async-load-calypso-components-web-preview-component                                     -38 B  (-0.0%)      -11 B  (-0.0%)
async-load-calypso-blocks-jitm-templates-sidebar-banner                                 -38 B  (-0.1%)      -12 B  (-0.1%)
async-load-calypso-blocks-jitm-templates-notice                                         -38 B  (-0.1%)      -12 B  (-0.1%)
async-load-calypso-blocks-jitm-templates-default                                        -38 B  (-0.1%)      -11 B  (-0.1%)
async-load-signup-steps-page-picker                                                     -19 B  (-0.0%)      -52 B  (-0.0%)
async-load-purchase-modal-wrapper                                                       -19 B  (-0.0%)      -39 B  (-0.0%)
async-load-my-sites-checkout-purchase-modal-is-eligible-for-one-click-checkou...        -19 B  (-0.0%)      -45 B  (-0.0%)
async-load-store-app-store-stats                                                        +18 B  (+0.0%)      +10 B  (+0.0%)
async-load-design-playground                                                            +18 B  (+0.0%)      +34 B  (+0.0%)
async-load-design                                                                       +18 B  (+0.0%)      +31 B  (+0.0%)

React components that are loaded lazily, when a certain part of UI is displayed for the first time.

Legend

What is parsed and gzip size?

Parsed Size: Uncompressed size of the JS and CSS files. This much code needs to be parsed and stored in memory.
Gzip Size: Compressed size of the JS and CSS files. This much data needs to be downloaded over network.

Generated by performance advisor bot at iscalypsofastyet.com.

eoigal

I think there is excess padding in the Appearance> Themes > Theme page

allilevine · 2024-08-30T14:03:17Z

I think there is excess padding in the Appearance> Themes > Theme page

@eoigal Thanks! The default top padding on layout__content is 79px. That plus NavigationHeader 24px top padding does look excessive, though I'm not sure what the standard is. I decided to match the individual Plugins page with 47px: 4213ee7

cc @javierarce in case you have a recommendation.

javierarce · 2024-08-30T14:18:44Z

I decided to match the individual Plugins page with 47px:

@allilevine that was a good solution.

I think we had a proposal to set the vertical distance to 24px, but since that would require changing at least the Plugins and the Domains pages, I think it's better to align Themes with Plugins for now.

allilevine · 2024-08-30T14:50:20Z

I think we had a proposal to set the vertical distance to 24px, but since that would require changing at least the Plugins and the Domains pages, I think it's better to align Themes with Plugins for now.

Thank you!

allilevine · 2024-08-30T17:21:56Z

I realized there's other paths to the theme page I need to allow for here. Putting this back in progress.

allilevine · 2024-09-05T21:39:59Z

client/my-sites/theme/main.jsx

 		const { backPath, locale, isLoggedIn, themeId } = this.props;
 		this.props.recordTracksEvent( 'calypso_theme_sheet_back_click', { theme_name: themeId } );

-		// Use history back when coming from customize your store screen.


Asked about removing this here: p1725572262533179-slack-C01SFMVEYAK

Confirmed: p1725630569410769/1725572262.533179-slack-C01SFMVEYAK

allilevine · 2024-09-06T13:53:52Z

client/my-sites/theme/main.jsx

-		}
-
-		page( localizeThemesPath( backPath, locale, ! isLoggedIn ) );
+		return localizeThemesPath( backPath, locale, ! isLoggedIn );


I'm returning a path for the breadcrumb instead of redirecting the page.

allilevine · 2024-09-06T15:19:31Z

client/components/breadcrumb/index.tsx

@@ -61,7 +61,7 @@ const StyledGridicon = styled( Gridicon )`
 	color: var( --color-neutral-10 );
 `;

-const HelpBuble = styled( InfoPopover )`
+const HelpBubble = styled( InfoPopover )`


Fixing a typo.

allilevine · 2024-09-06T15:19:56Z

client/components/breadcrumb/index.tsx

@@ -127,11 +132,13 @@ const Breadcrumb: React.FunctionComponent< Props > = ( props ) => {

 	return (
 		<StyledUl className="breadcrumbs">
-			{ items.map( ( item: { href?: string; label: string }, index: Key ) => (
+			{ items.map( ( item: Item, index: Key ) => (


Replacing the type here with the type already defined above.

allilevine · 2024-09-06T15:20:49Z

client/my-sites/theme/main.jsx


-		page( localizeThemesPath( backPath, locale, ! isLoggedIn ) );
+	handleBackLinkClick = () => {


I added this function for the Tracks event.

allilevine · 2024-09-06T15:24:51Z

client/my-sites/theme/main.jsx

@@ -1540,6 +1536,11 @@ class ThemeSheet extends Component {
 			'is-removed': isRemoved,
 		} );

+		const navigationItems = [
+			{ label: translate( 'Themes' ), href: this.getBackLink(), onClick: this.handleBackLinkClick },


I wonder if "Back to themes" was better language for screen readers. But this matches "Plugins."

DustyReagan

It looks good and works well. I noticed two things, but I don't think they're blockers.

/plugins on mobile view uses "< Back" instead of breadcrumbs. I'm not sure if we want to copy that behavior.

The header has some extra margin at a specific width. See video:

Screen.Recording.2024-09-06.at.4.56.21.PM.mov

allilevine · 2024-09-09T15:27:37Z

@DustyReagan Thanks for catching these!

/plugins on mobile view uses "< Back" instead of breadcrumbs. I'm not sure if we want to copy that behavior.

I noticed this but it comes from the compactBreadcrumb navigation header prop, which depends on breakpoint hooks. I can't use hooks in a class component, so I'd prefer to open a follow-up to look into it.

The header has some extra margin at a specific width. See video:

I pushed a change to fix this! If you can re-review I'd appreciate it. 🙏

I also noticed that there was a .is-mobile-app-view rule to hide the previous header on mobile apps. The Plugins header isn't hidden for apps, though. So that needs some investigation to find out whether it should be visible in the apps.

DustyReagan

I retested the new commit, and it looks good to me!

allilevine marked this pull request as ready for review August 29, 2024 20:28

allilevine self-assigned this Aug 29, 2024

eoigal reviewed Aug 30, 2024

View reviewed changes

allilevine force-pushed the add/theme-header-breadcrumbs branch from 4213ee7 to 09fcb97 Compare September 5, 2024 20:32

allilevine commented Sep 5, 2024

View reviewed changes

allilevine commented Sep 6, 2024

View reviewed changes

allilevine added the [Status] String Freeze Add the [Status] String Freeze label to your PR to ensure new strings are translated before merging label Sep 6, 2024

allilevine commented Sep 6, 2024

View reviewed changes

allilevine requested a review from eoigal September 6, 2024 15:49

matticbot added the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Sep 6, 2024

allilevine requested a review from a team September 6, 2024 15:49

DustyReagan approved these changes Sep 6, 2024

View reviewed changes

allilevine added 5 commits September 9, 2024 09:54

Change the theme header to breadcrumbs.

fa34d91

Reduce top layout padding for theme section.

10619ee

Use localizeThemesPath for breadcrumb URL.

d6b1003

Add tracks event on click.

fb981f2

Translate Themes breadcrumb.

b63f448

allilevine force-pushed the add/theme-header-breadcrumbs branch from bb8f8b2 to b63f448 Compare September 9, 2024 13:54

Fix box-sizing and remove header cake styles.

d2fbda6

allilevine requested a review from DustyReagan September 9, 2024 15:27

DustyReagan approved these changes Sep 9, 2024

View reviewed changes

allilevine merged commit 53acd87 into trunk Sep 9, 2024
12 checks passed

allilevine deleted the add/theme-header-breadcrumbs branch September 9, 2024 19:52

github-actions bot removed [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. [Status] String Freeze Add the [Status] String Freeze label to your PR to ensure new strings are translated before merging labels Sep 9, 2024

allilevine mentioned this pull request Sep 11, 2024

Theme Details: Add compact breadcrumb on mobile #94445

Merged

7 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Show breadcrumbs in individual theme header #94038

Show breadcrumbs in individual theme header #94038

allilevine commented Aug 29, 2024 •

edited

Loading

github-actions bot commented Aug 29, 2024 •

edited by matticbot

Loading

matticbot commented Aug 29, 2024 •

edited

Loading

matticbot commented Aug 29, 2024 •

edited

Loading

eoigal left a comment

allilevine commented Aug 30, 2024

javierarce commented Aug 30, 2024

allilevine commented Aug 30, 2024

allilevine commented Aug 30, 2024

allilevine Sep 5, 2024

allilevine Sep 6, 2024

allilevine Sep 6, 2024

allilevine Sep 6, 2024

allilevine Sep 6, 2024

allilevine Sep 6, 2024

allilevine Sep 6, 2024

DustyReagan left a comment •

edited

Loading

allilevine commented Sep 9, 2024

DustyReagan left a comment


		page( localizeThemesPath( backPath, locale, ! isLoggedIn ) );
		handleBackLinkClick = () => {

Show breadcrumbs in individual theme header #94038

Show breadcrumbs in individual theme header #94038

Conversation

allilevine commented Aug 29, 2024 • edited Loading

Proposed Changes

Why are these changes being made?

Testing Instructions

Pre-merge Checklist

github-actions bot commented Aug 29, 2024 • edited by matticbot Loading

matticbot commented Aug 29, 2024 • edited Loading

matticbot commented Aug 29, 2024 • edited Loading

eoigal left a comment

Choose a reason for hiding this comment

allilevine commented Aug 30, 2024

javierarce commented Aug 30, 2024

allilevine commented Aug 30, 2024

allilevine commented Aug 30, 2024

allilevine Sep 5, 2024

Choose a reason for hiding this comment

allilevine Sep 6, 2024

Choose a reason for hiding this comment

allilevine Sep 6, 2024

Choose a reason for hiding this comment

allilevine Sep 6, 2024

Choose a reason for hiding this comment

allilevine Sep 6, 2024

Choose a reason for hiding this comment

allilevine Sep 6, 2024

Choose a reason for hiding this comment

allilevine Sep 6, 2024

Choose a reason for hiding this comment

DustyReagan left a comment • edited Loading

Choose a reason for hiding this comment

allilevine commented Sep 9, 2024

DustyReagan left a comment

Choose a reason for hiding this comment

allilevine commented Aug 29, 2024 •

edited

Loading

github-actions bot commented Aug 29, 2024 •

edited by matticbot

Loading

matticbot commented Aug 29, 2024 •

edited

Loading

matticbot commented Aug 29, 2024 •

edited

Loading

DustyReagan left a comment •

edited

Loading