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.

Sign up for GitHub

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

Show breadcrumbs in individual theme header #94038

Merged
merged 6 commits into from
Sep 9, 2024
Merged

Show breadcrumbs in individual theme header #94038

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
fa34d91
Change the theme header to breadcrumbs.
allilevine Aug 29, 2024
10619ee
Reduce top layout padding for theme section.
allilevine Aug 30, 2024
d6b1003
Use localizeThemesPath for breadcrumb URL.
allilevine Sep 5, 2024
fb981f2
Add tracks event on click.
allilevine Sep 6, 2024
b63f448
Translate Themes breadcrumb.
allilevine Sep 6, 2024
d2fbda6
Fix box-sizing and remove header cake styles.
allilevine Sep 9, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion client/components/breadcrumb/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,6 @@ const BreadcrumbExamples = () => {

## Props

- `items` (`{ label: string; href?: string }[]`) - The Navigations items to be shown
- `items` (`{ label: string; href?: string; helpBubble?: React.ReactElement; onClick?: () => void }[]`) - The Navigations items to be shown
- `compact` (`boolean`) - Displays only the previous item URL (optional)
- `mobileItem` (`string`) - In compact version, displays this value. If not passed defaults to "Back" (optional)
19 changes: 13 additions & 6 deletions client/components/breadcrumb/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ const StyledGridicon = styled( Gridicon )`
color: var( --color-neutral-10 );
`;

const HelpBuble = styled( InfoPopover )`
const HelpBubble = styled( InfoPopover )`
Copy link
Member Author

Choose a reason for hiding this comment

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

Fixing a typo.

margin-left: 7px;
display: flex;
align-items: center;
Expand All @@ -79,13 +79,18 @@ const renderHelpBubble = ( item: Item ) => {
}

return (
<HelpBuble icon="help-outline" position="right">
<HelpBubble icon="help-outline" position="right">
{ item.helpBubble }
</HelpBuble>
</HelpBubble>
);
};

export type Item = { label: string; href?: string; helpBubble?: React.ReactElement };
export type Item = {
label: string;
href?: string;
helpBubble?: React.ReactElement;
onClick?: () => void;
};
interface Props {
items: Item[];
mobileItem?: Item;
Expand Down Expand Up @@ -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 ) => (
Copy link
Member Author

Choose a reason for hiding this comment

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

Replacing the type here with the type already defined above.

<StyledLi key={ index }>
{ index !== 0 && <StyledGridicon icon="chevron-right" size={ 14 } /> }
{ item.href && index !== items.length - 1 ? (
<a href={ item.href }>{ item.label }</a>
<a href={ item.href } onClick={ item.onClick }>
{ item.label }
</a>
) : (
<span>{ item.label }</span>
) }
Expand Down
3 changes: 2 additions & 1 deletion client/components/navigation-header/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

This component displays a header with a breadcrumb.
It can also include children items which will be positioned to the far right.
It will not show less than 2 items.

## How to use

Expand All @@ -20,7 +21,7 @@ function render() {

## Props

- `navigationItems` (`{ label: string; href: string }[]`) - The Navigations items to be shown
- `navigationItems` (`{ label: string; href?: string; helpBubble?: React.ReactElement; onClick?: () => void }[]`) - The Navigations items to be shown
- `id` (`string`) - ID for the header (optional)
- `className` (`string`) - A class name for the wrapped component (optional)
- `children` (`nodes`) – Any children elements which are being rendered to the far right (optional)
Expand Down
34 changes: 14 additions & 20 deletions client/my-sites/theme/main.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,9 @@ import QuerySitePlans from 'calypso/components/data/query-site-plans';
import QuerySitePurchases from 'calypso/components/data/query-site-purchases';
import QueryUserPurchases from 'calypso/components/data/query-user-purchases';
import SyncActiveTheme from 'calypso/components/data/sync-active-theme';
import HeaderCake from 'calypso/components/header-cake';
import InlineSupportLink from 'calypso/components/inline-support-link';
import Main from 'calypso/components/main';
import NavigationHeader from 'calypso/components/navigation-header';
import PremiumGlobalStylesUpgradeModal from 'calypso/components/premium-global-styles-upgrade-modal';
import ThemeSiteSelectorModal from 'calypso/components/theme-site-selector-modal';
import { THEME_TIERS } from 'calypso/components/theme-tier/constants';
Expand Down Expand Up @@ -1215,18 +1215,14 @@ class ThemeSheet extends Component {
return styleVariations.find( ( variation ) => variation.slug === selectedStyleVariationSlug );
};

goBack = () => {
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.
Copy link
Member Author

Choose a reason for hiding this comment

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

Asked about removing this here: p1725572262533179-slack-C01SFMVEYAK

Copy link
Member Author

Choose a reason for hiding this comment

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

Confirmed: p1725630569410769/1725572262.533179-slack-C01SFMVEYAK

const urlParams = new URLSearchParams( window.location.search );
if ( urlParams.has( 'from', 'customize-store' ) && window.history.length > 1 ) {
window.history.back();
return;
}
getBackLink = () => {
const { backPath, locale, isLoggedIn } = this.props;
return localizeThemesPath( backPath, locale, ! isLoggedIn );
Copy link
Member Author

Choose a reason for hiding this comment

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

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

};

page( localizeThemesPath( backPath, locale, ! isLoggedIn ) );
handleBackLinkClick = () => {
Copy link
Member Author

Choose a reason for hiding this comment

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

I added this function for the Tracks event.

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

getBannerUpsellTitle = () => <BannerUpsellTitle { ...this.props } />;
Expand Down Expand Up @@ -1377,6 +1373,11 @@ class ThemeSheet extends Component {
'is-removed': isRemoved,
} );

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

Choose a reason for hiding this comment

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

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

{ label: title },
];

return (
<Main className="theme__sheet">
<QueryCanonicalTheme themeId={ this.props.themeId } siteId={ siteId } />
Expand Down Expand Up @@ -1426,14 +1427,7 @@ class ThemeSheet extends Component {
/>
<ThanksModal source="details" themeId={ this.props.themeId } />
<ActivationModal source="details" />
<div className="theme__sheet-action-bar-container">
<HeaderCake
className="theme__sheet-action-bar"
backText={ translate( 'Back to themes' ) }
onClick={ this.goBack }
alwaysShowBackText
/>
</div>
<NavigationHeader navigationItems={ navigationItems } />
<div className={ columnsClassName }>
<div className="theme__sheet-column-header">
{ this.renderStagingPaidThemeNotice() }
Expand Down
35 changes: 12 additions & 23 deletions client/my-sites/theme/style.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ $button-border: 4px;

.layout.is-logged-in .layout__content {
overflow: clip;
padding-top: 47px;

.theme__sheet-screenshot,
.theme__sheet-web-preview {
Expand Down Expand Up @@ -61,6 +62,17 @@ $button-border: 4px;
overflow-y: auto;
}
}

.theme__sheet .navigation-header {
margin: 0 auto;
max-width: $theme-sheet-content-max-width;
padding: 24px 44px;

@media screen and (min-width: 960px) {
padding: 24px 20px;
box-sizing: initial;
}
}
}

.main.theme__sheet {
Expand Down Expand Up @@ -288,29 +300,6 @@ $button-border: 4px;
grid-area: preview;
}

.theme__sheet-action-bar-container {
box-shadow: 0 1px var(--color-border-subtle);
}

.header-cake.card.theme__sheet-action-bar {
box-shadow: none;
box-sizing: content-box;
margin: 0 auto;
max-width: $theme-sheet-content-max-width;
min-height: 70px;
padding: 0 20px;
position: relative;

@include breakpoint-deprecated( "<960px" ) {
margin: 0 0 32px 0;
min-height: 60px;
}
}

.is-mobile-app-view .header-cake.card.theme__sheet-action-bar {
display: none;
}

.theme__sheet-primary-button {
position: absolute;
top: 5px;
Expand Down
Loading