jsoneditor icon indicating copy to clipboard operation
jsoneditor copied to clipboard

Add ajv-i18n support

Open askalione opened this issue 4 years ago • 5 comments

Hello! Add i18n for validation error messges using ajv-i18n.

askalione avatar Feb 15 '21 14:02 askalione

Thanks for your PR Alexey, this will be a nice improvement.

The PR itself is very hard to see what actually changed: about every line in textmode.js and treemode.js is changed. Can you please maintain the 2 space indentation, and if you would like to do any refactorings in the code do that in a separate PR?

josdejong avatar Feb 17 '21 20:02 josdejong

Please note that you can run npm run lint to detect linting issues, and use npm run lint -- --fix to fix as much as possible automatically.

josdejong avatar Feb 17 '21 20:02 josdejong

and use npm run lint -- --fix to fix as much as possible automatically.

Thanks!

I have fixed indents issues and now all checks have passed. Should i make new PR?

askalione avatar Feb 17 '21 21:02 askalione

Thanks for the update, the code looks good 👍 . No need to create a new PR, I'll squash it when merging.

I'll test your work functionally soon.

josdejong avatar Feb 22 '21 19:02 josdejong

I've done some testing, and it works like a charm, thanks @askalione!

Two feedbacks:

  1. Bundling the full ajv-i18n is quite large: the minified+zipped bundle goes from 231KB to 244KB. I'm wondering whether we can come up with a solution which doesn't require bundling of all the locales. The only idea I can come up with is to have the user pass the right translation file himself via the options, like:

    const options = {
      language: 'ru'
      ajv-i18n: require('ajv-i18n/localize/ru')
    }
    

    But that is not easy to use. Do you have any ideas in this regard? If not I guess we simply have to accept this for now.

  2. JSONEditor has a minimalist version of the bundle which basically strips the Ace and Ajv, and color picker libraries. Since those are quite large. The ajv-i18n library is also stripped, but now the dist/jsoneditor-minimalist.js throws an error when you open the editor with a JSONSchema. We can solve this by putting a try/catch around the require loading ajv-i18n, the same way as is done for Ajv itself in the file tryRequireAjv.js. Can you adjust that and make sure the code doesn't break when ajv-i18n is not available in the bundle?

josdejong avatar Feb 28 '21 18:02 josdejong