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

Footer i18n: Update footer branding to be well supported in multiple languages #78018

Merged
merged 13 commits into from
Jul 17, 2023
Merged

Footer i18n: Update footer branding to be well supported in multiple languages #78018

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
9a94552
Theme SSR: Add a loading trigger to guarantee the same initial state …
WBerredo Jun 1, 2023
4003926
Revert "Theme SSR: Add a loading trigger to guarantee the same initia…
WBerredo Jun 9, 2023
d5079f5
Check the if the button should be disabled with a state variable
WBerredo Jun 9, 2023
fbc8b59
Use an empty state for Automattic branding before set the fianl value
WBerredo Jun 7, 2023
1425f4a
Update the XautomatticBrandingNoun from a hook to a normal method
WBerredo Jun 9, 2023
b58f085
Use preffix and suffix instead of article and noun
WBerredo Jun 9, 2023
3968c13
Replace Automattic's name from the translate string to a variable con…
WBerredo Jun 9, 2023
c9c9434
Add Automattic Brand during the translation retrival step
WBerredo Jun 12, 2023
5599cc7
Fix property name to comment
WBerredo Jun 12, 2023
a811943
Add the options in each translate
WBerredo Jun 12, 2023
399edfc
Merge branch 'trunk' into fix/footer-i18n
WBerredo Jun 13, 2023
4a52a98
Merge branch 'trunk' into fix/footer-i18n
WBerredo Jun 29, 2023
4223643
Fix lint errors
WBerredo Jun 29, 2023
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
14 changes: 12 additions & 2 deletions client/my-sites/theme/main.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,12 @@ class ThemeSheet extends Component {
section: '',
};

/**
* Disabled button checks `isLoading` to determine if a the buttons should be disabled
* Its assigned to state to guarantee the initial state will be the same for SSR
*/
state = {
disabledButton: true,
showUnlockStyleUpgradeModal: false,
};

Expand All @@ -173,6 +178,11 @@ class ThemeSheet extends Component {
if ( this.props.themeId !== prevProps.themeId ) {
this.scrollToTop();
}

if ( this.state.disabledButton !== this.isLoading() ) {
// eslint-disable-next-line react/no-did-update-set-state
this.setState( { disabledButton: this.isLoading() } );
}
}

isLoaded = () => {
Expand Down Expand Up @@ -945,7 +955,7 @@ class ThemeSheet extends Component {
this.onButtonClick();
} }
primary
disabled={ this.isLoading() }
disabled={ this.state.disabledButton }
target={ isActive ? '_blank' : null }
>
{ this.isLoaded() ? label : placeholder }
Expand Down Expand Up @@ -1218,7 +1228,7 @@ class ThemeSheet extends Component {
}

const upsellNudgeClasses = classNames( 'theme__page-upsell-banner', {
'theme__page-upsell-disabled': this.isLoading(),
'theme__page-upsell-disabled': this.state.disabledButton,
} );

if ( hasWpComThemeUpsellBanner ) {
Expand Down
27 changes: 0 additions & 27 deletions packages/wpcom-template-parts/src/hooks/use-automattic-branding-noun.ts
View file
Open in desktop

This file was deleted.

4 changes: 2 additions & 2 deletions packages/wpcom-template-parts/src/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import type useAutomatticBrandingNoun from './hooks/use-automattic-branding-noun';
import type { getAutomatticBrandingNoun } from './utils';
import type { useLocalizeUrl } from '@automattic/i18n-utils';

export interface HeaderProps {
Expand All @@ -19,7 +19,7 @@ export interface PureFooterProps extends FooterProps {
localizeUrl?: ReturnType< typeof useLocalizeUrl >;
locale?: string;
isEnglishLocale?: boolean;
automatticBranding?: ReturnType< typeof useAutomatticBrandingNoun >;
automatticBranding?: ReturnType< typeof getAutomatticBrandingNoun >;
languageOptions?: LanguageOptions;
}

Expand Down
19 changes: 13 additions & 6 deletions packages/wpcom-template-parts/src/universal-footer-navigation/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,10 @@ import {
useLocalizeUrl,
} from '@automattic/i18n-utils';
import { __ } from '@wordpress/i18n';
import { useTranslate } from 'i18n-calypso';
import { useLayoutEffect, useState } from 'react';
import SocialLogo from 'social-logos';
import useAutomatticBrandingNoun from '../hooks/use-automattic-branding-noun';
import { getAutomatticBrandingNoun } from '../utils';
import type { FooterProps, PureFooterProps, LanguageOptions } from '../types';

import './style.scss';
Expand Down Expand Up @@ -58,8 +60,8 @@ export const PureUniversalNavbarFooter = ( {
onLanguageChange = defaultOnLanguageChange,
localizeUrl = pureLocalizeUrl,
automatticBranding = {
article: __( 'An', __i18n_text_domain__ ),
noun: __( 'thingamajig', __i18n_text_domain__ ),
prefix: __( 'An', __i18n_text_domain__ ),
suffix: __( 'thingamajig', __i18n_text_domain__ ),
},
locale,
languageOptions = allLanguageOptions,
Expand Down Expand Up @@ -434,7 +436,7 @@ export const PureUniversalNavbarFooter = ( {
<div className="lpc-footer-automattic-nav">
<div className="lpc-footer-automattic-nav-wrapper">
<a className="lp-logo-label" href="https://automattic.com/">
{ automatticBranding.article }
{ automatticBranding.prefix }
<span className="lp-hidden">Automattic</span>
<svg
className="lp-icon lp-icon--custom-automattic-footer"
Expand All @@ -444,7 +446,7 @@ export const PureUniversalNavbarFooter = ( {
>
<path d="M121 .68c1.9 0 3.62.82 4.55 1.86l-1.05 1.1c-.81-.77-2-1.5-3.6-1.5-2.4 0-3.75 1.71-3.75 3.48v.19c0 1.76 1.36 3.4 3.87 3.4 1.5 0 2.74-.74 3.52-1.5l1.01 1.11a6.58 6.58 0 0 1-4.64 1.86c-3.4 0-5.46-2.29-5.46-4.8v-.31c0-2.52 2.25-4.89 5.54-4.89zm-104.64.34v5.46c0 1.71 1.09 2.73 3.17 2.73 2.13 0 3-1.02 3-2.73V1.02h1.69v5.43c0 2.3-1.43 4.23-4.82 4.23-3.22 0-4.72-1.82-4.72-4.23V1h1.68zM45.88.68c3.2 0 5.25 2.33 5.25 4.85v.3c0 2.48-2.06 4.85-5.26 4.85-3.18 0-5.24-2.37-5.24-4.85v-.3C40.63 3 42.69.68 45.88.68zm-8.35.34v1.45H33.6v7.85h-1.68V2.47h-3.93V1.02h9.54zm20.12 0 3.54 6.38.42.78.42-.78 3.5-6.4h2.31v9.3h-1.68V2.97l-.45.8-3.76 6.56h-.82L57.4 3.77l-.45-.81v7.36h-1.64v-9.3h2.33zm35.47 0v1.45h-3.93v7.85h-1.68V2.47h-3.93V1.02h9.54zm12.36 0v1.45h-3.92v7.85h-1.69V2.47h-3.92V1.02h9.53zm5.82 0v9.3h-1.66V1.89c.67 0 .94-.37.94-.88h.72zm-104.5 0 4.94 9.3h-1.8l-1.19-2.3H3.48l-1.15 2.3H.55l4.86-9.3h1.4zm70.66 0 4.93 9.3h-1.8l-1.19-2.3h-5.27l-1.15 2.3H71.2l4.86-9.3h1.4zM45.88 2.15c-2.3 0-3.55 1.6-3.55 3.4v.23c0 1.8 1.25 3.43 3.55 3.43 2.29 0 3.56-1.63 3.56-3.43v-.23c0-1.8-1.27-3.4-3.56-3.4zm1.1 1.77a.7.7 0 0 1 .2.94l-1.54 2.46a.64.64 0 0 1-.9.2.7.7 0 0 1-.2-.93l1.54-2.47a.64.64 0 0 1 .9-.2zM6.08 2.83 4.1 6.74h3.98l-2.02-3.9zm70.65 0-1.96 3.91h3.98l-2.02-3.9z"></path>
</svg>
{ automatticBranding.noun }
{ automatticBranding.suffix }
</a>
<div className="lp-logo-label-spacer"></div>
<a className="lp-link-work" href="https://automattic.com/work-with-us/">
Expand All @@ -471,10 +473,15 @@ const UniversalNavbarFooter = ( {
}: FooterProps ) => {
const localizeUrl = useLocalizeUrl();
const locale = useLocale();
const translate = useTranslate();
const isEnglishLocale = useIsEnglishLocale();
const pathNameWithoutLocale =
currentRoute && removeLocaleFromPathLocaleInFront( currentRoute ).slice( 1 );
const automatticBranding = useAutomatticBrandingNoun();
const [ automatticBranding, setAutomatticBranding ] = useState( { prefix: '', suffix: '' } );

useLayoutEffect( () => {
setAutomatticBranding( getAutomatticBrandingNoun( translate ) );
}, [ translate ] );

return (
<PureUniversalNavbarFooter
Expand Down
30 changes: 30 additions & 0 deletions packages/wpcom-template-parts/src/utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
import { translate as translateMethod } from 'i18n-calypso';

export const getAutomatticBrandingNoun = ( translate: typeof translateMethod ) => {
const AutomatticBrand = 'Automattic';
const options = {
description:
'Branding to be shown on the Footer of the page, Automattic\'s variable will always contains the word "Automattic"',
args: { Automattic: AutomatticBrand },
};
const automatticRoger = [
translate( 'An %(Automattic)s brainchild', options ),
translate( 'An %(Automattic)s contraption', options ),
translate( 'An %(Automattic)s creation', options ),
translate( 'An %(Automattic)s experiment', options ),
translate( 'An %(Automattic)s invention', options ),
translate( 'An %(Automattic)s joint', options ),
translate( 'An %(Automattic)s medley', options ),
translate( 'An %(Automattic)s opus', options ),
translate( 'An %(Automattic)s production', options ),
translate( 'An %(Automattic)s ruckus', options ),
translate( 'An %(Automattic)s thingamajig', options ),
WBerredo marked this conversation as resolved.
Show resolved Hide resolved
];

const branding = automatticRoger[ Math.floor( Math.random() * ( 10 - 0 + 1 ) + 0 ) ]?.toString();
Copy link
Member

Choose a reason for hiding this comment

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

I'm wondering what these ±0 arithmetics are supposed to achieve. To select a random array element, seems to me that simple roger[ Math.floor( Math.random() * roger.length ) ] is sufficient.

Also, once you avoid the .split call and use interpolation to insert the A8c logo, the .toString call is no longer needed. In theory branding can be a HTML/JSX markup then.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm wondering what these ±0 arithmetics are supposed to achieve. To select a random array element, seems to me that simple roger[ Math.floor( Math.random() * roger.length ) ] is sufficient.

Agreed, I'm also not sure why are they needed, but I suppose it must be needed as it is a non-trivial addition.


return {
prefix: branding.split( 'Automattic' )[ 0 ],
suffix: branding.split( 'Automattic' )[ 1 ],
};
};