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

fix: add translateble false to generated android strings #896

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

emartynov
Copy link

@emartynov emartynov commented Nov 17, 2022

#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

Just to make our lint happy
@emartynov
Copy link
Author

This probably will be never merged

@emartynov emartynov changed the title Add translateble false to generated android strings fix: add translateble false to generated android strings Mar 19, 2023
@chazzmoney
Copy link
Collaborator

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
Copy link
Author

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
Copy link
Collaborator

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
Copy link
Collaborator

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

'android/resources': function({dictionary, options, file}) {
const template = _template(
fs.readFileSync(__dirname + '/templates/android/resources.template')
);
return template({dictionary, file, options, fileHeader});
},

@chazzmoney
Copy link
Collaborator

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants