Skip to content

Move constant to fix translations #591

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

Merged
wojtekn merged 4 commits into from
Oct 11, 2024
Merged

Move constant to fix translations #591

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
55ce7bb
Move constant to fix translations
wojtekn Oct 10, 2024
d07d306
Add more alphabets to characters allowed for path
wojtekn Oct 10, 2024
5affe3b
Fix lint issue
wojtekn Oct 10, 2024
52af16c
Add Korean letters (Jamo) to allowed list as normalization decomposes…
wojtekn Oct 11, 2024
File filter

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
74 changes: 45 additions & 29 deletions src/lib/generate-site-name.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,29 +1,28 @@
import { __ } from '@wordpress/i18n';

const siteNames = [
__( 'My Bold Website' ),
__( 'My Bright Website' ),
__( 'My Blissful Website' ),
__( 'My Calm Website' ),
__( 'My Cool Website' ),
__( 'My Dreamy Website' ),
__( 'My Elite Website' ),
__( 'My Fresh Website' ),
__( 'My Glowing Website' ),
__( 'My Happy Website' ),
__( 'My Joyful Website' ),
__( 'My Noble Website' ),
__( 'My Pure Website' ),
__( 'My Peak Website' ),
__( 'My Prime Website' ),
__( 'My Serene Website' ),
__( 'My Shiny Website' ),
__( 'My Sparkly Website' ),
__( 'My Swift Website' ),
__( 'My True Website' ),
];

export function generateSiteName( usedSiteNames: string[] ): string {
const siteNames = [
__( 'My Bold Website' ),
__( 'My Bright Website' ),
__( 'My Blissful Website' ),
__( 'My Calm Website' ),
__( 'My Cool Website' ),
__( 'My Dreamy Website' ),
__( 'My Elite Website' ),
__( 'My Fresh Website' ),
__( 'My Glowing Website' ),
__( 'My Happy Website' ),
__( 'My Joyful Website' ),
__( 'My Noble Website' ),
__( 'My Pure Website' ),
__( 'My Peak Website' ),
__( 'My Prime Website' ),
__( 'My Serene Website' ),
__( 'My Shiny Website' ),
__( 'My Sparkly Website' ),
__( 'My Swift Website' ),
__( 'My True Website' ),
];
let proposedName = __( 'My WordPress Website' );
let tryCount = 0;

Expand All @@ -36,14 +35,31 @@ export function generateSiteName( usedSiteNames: string[] ): string {
}

export const sanitizeFolderName = ( filename: string ) => {
const LATIN = 'a-z';
const CYRILLIC = 'а-яё';
const ARABIC = '\\u0600-\\u06FF';
const HEBREW = '\\u0590-\\u05FF';
const CHINESE = '\\u4e00-\\u9fa5';
const JAPANESE_HIRAGANA = '\\u3040-\\u309F';
const JAPANESE_KATAKANA = '\\u30A0-\\u30FF';
const KOREAN_HANGUL = '\\uAC00-\\uD7AF';

@wojtekn wojtekn Oct 10, 2024

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The sanitization doesn't work for a Korean as NFKD normalization removes all Korean characters, producing an empty path.

Any smart way on how to solve that?

@wojtekn wojtekn Oct 10, 2024

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Potential ways:

  • Don't normalize string if a user uses Studio in Korean. It's an easy way, but edge cases won't work well e.g. user who uses Studio in Korean and uses Polish characters in the site name.
  • replace Korean characters with placeholders before normalization and replace them back after. It would work in all cases, but seems overly complex.

@sejas sejas Oct 11, 2024

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Yeah, I think we can skip normalization if Studio is in Korean.

@wojtekn wojtekn Oct 11, 2024

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It seems I debugged it incorrectly and came to the wrong conclusion.

Korean syllables (Hangul) are decomposed by NKFD normalization into letters (Jamo). The initial regex included only syllables, so all letters were removed. I added Korean letters to the allowed list, which now works for all cases.

@katinthehatsite katinthehatsite Oct 11, 2024

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for the fix, it looks good for me now 👍

I added Korean letters to the allowed list, which now works for all cases.

Nice work!
const KOREAN_JAMO = '\\u1100-\\u11FF'; // Hangul syllables are decomposed to Jamo letters
const NUMBERS = '0-9';
const WHITELISTED_SYMBOLS = '_\\- '; // Allow underscore, hyphen, and space

const ALLOWED_CHARS = new RegExp(
`[^${ LATIN }${ NUMBERS }${ CYRILLIC }${ ARABIC }${ HEBREW }${ CHINESE }${ JAPANESE_HIRAGANA }${ JAPANESE_KATAKANA }${ KOREAN_HANGUL }${ KOREAN_JAMO }${ WHITELISTED_SYMBOLS }]`,
'gi'
);

return String( filename )
.replace( /ł/g, 'l' )
.replace( /Ł/g, 'L' )
.replace( /ł/g, 'l' ) // Polish ł to l
.replace( /Ł/g, 'L' ) // Polish Ł to L
.normalize( 'NFKD' )
.replace( /[\u0300-\u036f]/g, '' )
.replace( /[\u0300-\u036f]/g, '' ) // Remove diacritics
.toLowerCase()
.replace( /[^a-z0-9 -]/g, '' )
.replace( ALLOWED_CHARS, '' )
.trim()
.replace( /\s+/g, '-' )
.replace( /-+/g, '-' );
.replace( /\s+/g, '-' ) // Replace spaces with hyphens
.replace( /-+/g, '-' ); // Replace multiple hyphens with a single one
};