vendure icon indicating copy to clipboard operation
vendure copied to clipboard

feat(core,common,admin-ui): Implement normalizeString function using slug library

Open andriinuts opened this issue 2 years ago • 5 comments

Description

Updated normalizeString to use slug npm package which fixes issues with multilanguage slug generation and especially Cyrillic letters: https://discord.com/channels/1100672177260478564/1208130249632649256

Breaking changes

Updated all calls of the normalizeString function to include a third parameter for language code. This enhancement allows the function to normalize strings based on specific language rules.

Checklist

📌 Always:

  • [x] I have set a clear title
  • [x] My PR is small and contains a single feature
  • [x] I have checked my own PR

👍 Most of the time:

  • [x] I have added or updated test cases
  • [x] I have updated the README if needed

andriinuts avatar Mar 07 '24 17:03 andriinuts

Deploy Preview for effervescent-donut-4977b2 ready!

Name Link
Latest commit 971658e34b6f7a729f1b9e662af300c09dcb83ce
Latest deploy log https://app.netlify.com/sites/effervescent-donut-4977b2/deploys/65e9f5af6484580008023b85
Deploy Preview https://deploy-preview-2721--effervescent-donut-4977b2.netlify.app
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

netlify[bot] avatar Mar 07 '24 17:03 netlify[bot]

Hi,

This is basically good and I like the slug lib since it is small and has zero dependencies. The main issue I see is the way it is handling non-supported languages, e.g. Chinese. Right now it is replacing any Chinese string with '', which is also why the tests are failing.

In the Discord chat about this, a Chinese user said he would in any case use English for the slugs, but of course it is not feasible to translate Chinese to English in a slug lib!

So my preferred behaviour would be to leave the chinses as-is, rather than replacing with empty string. For the Admin UI this is not a big issue since the admin can directly edit the slug before saving. But for things like imports, we could run into the problem.

Do you have any ideas on this?

michaelbromley avatar Mar 18 '24 11:03 michaelbromley

Hi,

This is basically good and I like the slug lib since it is small and has zero dependencies. The main issue I see is the way it is handling non-supported languages, e.g. Chinese. Right now it is replacing any Chinese string with '', which is also why the tests are failing.

In the Discord chat about this, a Chinese user said he would in any case use English for the slugs, but of course it is not feasible to translate Chinese to English in a slug lib!

So my preferred behaviour would be to leave the chinses as-is, rather than replacing with empty string. For the Admin UI this is not a big issue since the admin can directly edit the slug before saving. But for things like imports, we could run into the problem.

Do you have any ideas on this?

Hi, I agree. But for slug I haven't found a solution on how to leave just Chinese characters. I guess for this we have to add all characters to the charmap the same as I did for the - like: slug.charmap['-'] = '-'; or slug.extend({'-': '-'}) but Chinese json will be huge (we will replace English to the same character, what I don't like too much). Or maybe we can detect the Chinese language by regexp /[\u4e00-\u9fa5]/.test(text) and use your previous logic for this case and slug for all other languages. What do you think?

andriinuts avatar Mar 19 '24 19:03 andriinuts

@michaelbromley do you have any thoughts?

andriinuts avatar Apr 17 '24 13:04 andriinuts

@andriinuts Yes, if we can have the benefit of 'slug' for the western languages, and no-op for non-supported languages like Chinese, then this is a good approach.

michaelbromley avatar Apr 25 '24 11:04 michaelbromley