Rephrase usage of Palette in user-facing strings#77
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the UI terminology by replacing references to "Palette" with "Brushes" or "Cahier" across the Brush Designer and Brush Graph modules, while also applying some code reformatting for better readability. The review feedback identifies significant redundancy in the newly added string resources. Specifically, the reviewer suggests consolidating module-specific strings for "Save" and "My Brushes" into existing generic or shared resources to reduce localization overhead and improve maintainability.
| <string name="brush_designer_my_brushes">My Brushes</string> | ||
| <string name="brush_designer_no_saved_brushes">No saved brushes yet</string> | ||
| <string name="brush_designer_save_to_palette">Save to Cahier Palette</string> | ||
| <string name="brush_designer_save">Save</string> |
There was a problem hiding this comment.
The string resource brush_designer_save is redundant because it has the same value ('Save') as the generic save string defined on line 43. Since R.string.save is already used in other parts of the application (including within this same module's dialogs), it is better to use the existing generic resource to maintain consistency and reduce localization overhead.
| <string name="bg_save">Save</string> | ||
| <string name="bg_save_to_cahier">Save to Cahier</string> |
There was a problem hiding this comment.
There is redundancy between the bg_ prefixed strings and the brush_designer_ prefixed strings. Specifically:
bg_save(line 281) is identical tobrush_designer_save(line 110) and the genericsave(line 43).bg_save_to_cahier(line 282) is identical tobrush_designer_save_dialog_title(line 114).bg_my_brushes(line 311) is identical tobrush_designer_my_brushes(line 108).
While module-specific prefixes can be useful, for strings that are identical and used across multiple developer tools, it is more maintainable to define a single shared resource (e.g., my_brushes or save_to_cahier) in a common section.
| } else { | ||
| TextButton(onClick = onShowSaveDialog) { | ||
| Text(stringResource(R.string.brush_designer_save_to_palette)) | ||
| Text(stringResource(R.string.brush_designer_save)) |
There was a problem hiding this comment.
Consider using the generic R.string.save instead of the module-specific R.string.brush_designer_save to avoid redundancy, as they both resolve to the same value and the generic one is already used elsewhere in this file (e.g., line 418).
| Text(stringResource(R.string.brush_designer_save)) | |
| Text(stringResource(R.string.save)) |
| modifier = Modifier.height(40.dp), | ||
| ) { | ||
| Text(stringResource(R.string.bg_save_to_palette)) | ||
| Text(stringResource(R.string.bg_save)) |
There was a problem hiding this comment.
Consider using the generic R.string.save instead of R.string.bg_save to reduce string resource redundancy.
| Text(stringResource(R.string.bg_save)) | |
| Text(stringResource(R.string.save)) |
2 participants
"Palette" is a bit confusing in English when what we mean is "a collection of brushes". In English, "palette" generally refers to either a collection of colors or a board which a painter would use to hold a collection of different colored paints. This change replaces "Save to Palette" with "Save" on the button (and "Save to Cahier" in the dialog box) and "My Palette" with "My Brushes".
There's still some functions referring to the palette, but since that's not user-facing I think it's ok for now, and lower priority to do a larger renaming refactor of the code.