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.

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

Update/i18n utils unreverted #47492

Closed
wants to merge 10 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
8 changes: 7 additions & 1 deletion client/components/calypso-i18n-provider/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,12 @@
import * as React from 'react';
import { setLocaleData as setWpI18nLocaleData } from '@wordpress/i18n';
import { I18nProvider } from '@automattic/react-i18n';
import { LocaleProvider } from '@automattic/i18n-utils';
import i18n from 'i18n-calypso';

const CalypsoI18nProvider: React.FunctionComponent = ( { children } ) => {
const [ localeData, setLocaleData ] = React.useState( i18n.getLocale() );
const localeSlug = i18n.getLocaleSlug();

React.useEffect( () => {
const onChange = () => {
Expand All @@ -25,7 +27,11 @@ const CalypsoI18nProvider: React.FunctionComponent = ( { children } ) => {
};
}, [] );

return <I18nProvider localeData={ localeData }>{ children }</I18nProvider>;
return (
<LocaleProvider localeSlug={ localeSlug || 'en' }>
Copy link
Member

Choose a reason for hiding this comment

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

Let's use config( 'i18n_default_locale_slug' ) for fallback instead of 'en' string literal.

Copy link
Member

Choose a reason for hiding this comment

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

Great, TIL! There's some other places I can use this config value too 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Exposed and used the hard coded one in i18n-utils, I think we need to strip these i18n config values from calypso usage and rely on the utils package unless we want to configure LocaleProvider with the configuration as well?

This is done in #47613

<I18nProvider localeData={ localeData }>{ children }</I18nProvider>
</LocaleProvider>
);
lsl marked this conversation as resolved.
Show resolved Hide resolved
};

export default CalypsoI18nProvider;
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import { useSelect } from '@wordpress/data';
import { Icon, chevronDown } from '@wordpress/icons';
import { sprintf } from '@wordpress/i18n';
import { useI18n } from '@automattic/react-i18n';
import { useLocale } from '@automattic/i18n-utils';

/**
* Internal dependencies
Expand All @@ -22,7 +23,8 @@ import { DOMAIN_SUGGESTIONS_STORE } from '../../stores/domain-suggestions';
import './style.scss';

const DomainPickerButton: React.FunctionComponent = () => {
const { __, i18nLocale } = useI18n();
const { __ } = useI18n();
const locale = useLocale();
const makePath = usePath();
const { domain, selectedDesign, siteTitle, siteVertical } = useSelect( ( select ) =>
select( ONBOARD_STORE ).getState()
Expand All @@ -43,7 +45,7 @@ const DomainPickerButton: React.FunctionComponent = () => {
include_wordpressdotcom: false,
include_dotblogsubdomain: false,
quantity: 1, // this will give the recommended domain only
locale: i18nLocale,
locale,
} );
},
[ suggestionQuery ]
Expand Down
1 change: 1 addition & 0 deletions packages/domain-picker/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
"@automattic/data-stores": "^1.0.0-alpha.1",
"@automattic/onboarding": "^1.0.0",
"@automattic/react-i18n": "^1.0.0-alpha.0",
"@automattic/i18n-utils": "^1.0.0",
"@wordpress/base-styles": "^2.0.1",
"@wordpress/components": "^10.0.5",
"@wordpress/compose": "^3.19.3",
Expand Down
9 changes: 6 additions & 3 deletions packages/domain-picker/src/domain-picker/suggestion-item.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
* External dependencies
*/
import React, { FunctionComponent, useEffect, useState } from 'react';
import { __ } from '@wordpress/i18n';

import { createInterpolateElement } from '@wordpress/element';
import { Spinner } from '@wordpress/components';
import { useViewportMatch } from '@wordpress/compose';
Expand All @@ -11,7 +11,8 @@ import { sprintf } from '@wordpress/i18n';
import { v4 as uuid } from 'uuid';
import { recordTrainTracksInteract } from '@automattic/calypso-analytics';
import { Button } from '@wordpress/components';

import { useI18n } from '@automattic/react-i18n';
import { useLocalizeUrl } from '@automattic/i18n-utils';
/**
* Internal dependencies
*/
Expand Down Expand Up @@ -58,6 +59,8 @@ const DomainPickerSuggestionItem: FunctionComponent< Props > = ( {
selected,
type = ITEM_TYPE_RADIO,
} ) => {
const { __ } = useI18n();
const localizeUrl = useLocalizeUrl();
const isMobile = useViewportMatch( 'small', '<' );

const dotPos = domain.indexOf( '.' );
Expand Down Expand Up @@ -162,7 +165,7 @@ const DomainPickerSuggestionItem: FunctionComponent< Props > = ( {
<a
target="_blank"
rel="noreferrer"
href="https://wordpress.com/support/https-ssl"
href={ localizeUrl( 'https://wordpress.com/support/https-ssl' ) }
/>
), // TODO Wrap this in `localizeUrl` from lib/i18n-utils
}
Expand Down
3 changes: 2 additions & 1 deletion packages/i18n-utils/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
"test": "yarn jest"
},
"dependencies": {
"i18n-calypso": "^5.0.0"
"@testing-library/react-hooks": "^3.4.2",
"react": "^16.12.0"
}
}
6 changes: 5 additions & 1 deletion packages/i18n-utils/src/index.ts
Original file line number Diff line number Diff line change
@@ -1 +1,5 @@
export { localizeUrl } from './localize-url';
/**
* Internal dependencies
*/
export { localizeUrl, useLocalizeUrl } from './localize-url';
export { LocaleProvider, useLocale } from './locale-context';
18 changes: 18 additions & 0 deletions packages/i18n-utils/src/locale-context.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
/**
* External dependencies
*/
import React, { createContext, useContext } from 'react';

export const LocaleContext = createContext< string >( 'en' );

interface Props {
localeSlug: string;
}

export const LocaleProvider: React.FC< Props > = ( { children, localeSlug } ) => (
<LocaleContext.Provider value={ localeSlug }>{ children }</LocaleContext.Provider>
);

export function useLocale(): string {
return useContext( LocaleContext );
}
19 changes: 13 additions & 6 deletions packages/i18n-utils/src/localize-url.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
/**
* Internal dependencies
*/
import { getLocaleSlug } from 'i18n-calypso';

import {
localesWithBlog,
localesWithPrivacyPolicy,
Expand All @@ -14,6 +12,7 @@ import {
jetpackComLocales,
Locale,
} from './locales';
import { useLocale } from './locale-context';

const INVALID_URL = `http://__domain__.invalid`;

Expand Down Expand Up @@ -86,10 +85,7 @@ const urlLocalizationMapping: UrlLocalizationMapping = {
'wordpress.com': setLocalizedUrlHost( 'wordpress.com', magnificentNonEnLocales ),
};

export function localizeUrl( fullUrl: string, toLocale?: Locale ): string {
const locale =
toLocale || ( typeof getLocaleSlug === 'function' ? getLocaleSlug() || 'en' : 'en' );

export function localizeUrl( fullUrl: string, locale: Locale ): string {
const url = new URL( String( fullUrl ), INVALID_URL );

// Ignore and passthrough /relative/urls that have no host specified
Expand Down Expand Up @@ -132,3 +128,14 @@ export function localizeUrl( fullUrl: string, toLocale?: Locale ): string {
// Nothing needed to be changed, just return it unmodified.
return fullUrl;
}

export function useLocalizeUrl(): ( fullUrl: string, locale?: Locale ) => string {
Copy link
Member

Choose a reason for hiding this comment

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

👍

const providerLocale = useLocale();

return ( fullUrl: string, locale?: Locale ) => {
lsl marked this conversation as resolved.
Show resolved Hide resolved
if ( locale ) {
return localizeUrl( fullUrl, locale );
}
return localizeUrl( fullUrl, providerLocale );
};
}
Loading