amzn / style-dictionary

A build system for creating cross-platform styles.
https://styledictionary.com
Apache License 2.0
3.87k stars 543 forks source link

fix: add translateble false to generated android strings #896

Open emartynov opened 1 year ago

emartynov commented 1 year ago

#893

Description of changes:

Add translatable="false" attribute to the generated strings. I assume it is default and only one correct use of these strings.

Android documentation is here http://tools.android.com/recent/non-translatablestrings

It would be also great to add test for it here https://github.com/amzn/style-dictionary/blob/main/__tests__/formats/androidResources.test.js

emartynov commented 1 year ago

This probably will be never merged

chazzmoney commented 1 year ago

Two quick questions:

  1. Is there potential for a situation where the string should not have this attribute? Like somehow someone uses SD for an actual message to be used for ux display to the user?
  2. Can you add the test?

I think if we had the test and the answer to the question, it would make this actionable.

emartynov commented 1 year ago

For the first question, I can not say for sure. Is there an option to make it configurable? I will try to write tests. However, my typescript knowledge is limited. I will try to mimic some existing tests.

chazzmoney commented 1 year ago

I think it is likely that someone is using SD to have multi-platform translatable strings, including strings on Android. To avoid breaking any users who might be using SD in this way, this functionality should be enabled by configuration.

chazzmoney commented 1 year ago

config options are passed in to this template via the function here:

https://github.com/amzn/style-dictionary/blob/b0a89ca766af6891b0d731a047820693d65c9bf1/lib/common/formats.js#L538-L543

chazzmoney commented 1 year ago

Its maybe a weird moment; we have lots of config but not much (I think none?) config specific to a platform / output format. This is the documentation for config:

https://github.com/amzn/style-dictionary/blob/b0a89ca766af6891b0d731a047820693d65c9bf1/docs/config.md

We have config for platforms and for individual files, but we don't have any info about documenting options for platform specific configurations. We will have to add something I think.