JNG-5946 fix row delete translation #465
Conversation
noherczeg
commented
Oct 1, 2024
•
noherczeg
commented
 JNG-5946 Translation of delete operation is missing from tables
|
noherczeg
requested review from
robertcsakany,
bencegelei,
gaborflorian and
Madmat8
WalkthroughThe changes enhance the Judo UI React application by introducing new translation keys specifically for row deletion functionality. Modifications were made to the Changes
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🔇 Additional comments (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (5)
judo-ui-react/src/main/resources/actor/public/i18n/system_default.json.hbs (1)
24-25: LGTM! Consider a minor adjustment for consistency.The new translation entries for row deletion are well-placed and consistent with the existing structure. They align perfectly with the PR objective of fixing row delete translation.
For improved consistency with other action translations, consider changing "Delete successful" to "Row delete successful". This would make it more specific and in line with the "row-delete" action.
Here's a suggested minor adjustment:
"judo.action.row-delete": "Delete", -"judo.action.row-delete.success": "Delete successful", +"judo.action.row-delete.success": "Row delete successful",judo-ui-react/src/main/resources/actor/public/i18n/system_en-US.json.hbs (1)
24-25: LGTM! Consider a minor enhancement for clarity.The new translation keys "judo.action.row-delete" and "judo.action.row-delete.success" are well-integrated and consistent with the existing translations. They follow the established naming conventions and provide clear, concise translations.
To further improve clarity and maintain consistency with other similar keys, consider the following minor enhancement:
Consider changing "judo.action.row-delete.success" to "Row delete successful" instead of "Delete successful". This would make it more specific and consistent with the "row" context, similar to how you have "judo.action.create.success" as "Create successful".
"judo.action.row-delete": "Delete", -"judo.action.row-delete.success": "Delete successful", +"judo.action.row-delete.success": "Row delete successful",This change would make the success message more explicit about the specific action that was successful.
judo-ui-react/src/main/resources/actor/public/i18n/system_hu-HU.json.hbs (1)
24-25: LGTM! Consider a minor enhancement for clarity.The new translation entries for row deletion are correct and consistent with the existing translations. They follow the established naming conventions and provide clear, concise Hungarian translations for the delete action and success message.
For even greater clarity, you might consider making the success message more specific to row deletion:
"judo.action.row-delete": "Törlés", -"judo.action.row-delete.success": "Sikeres törlés" +"judo.action.row-delete.success": "Sor sikeresen törölve"This change would translate to "Row successfully deleted" in English, which might provide more context. However, the current translation is also acceptable and consistent with other success messages in the file.
judo-ui-react/src/main/java/hu/blackbelt/judo/ui/generator/react/UiI18NHelper.java (2)
180-181: LGTM! Consider adding a constant for the translation key.The addition of the
row-deleteaction handling is correct and follows the existing pattern. For consistency with other parts of the codebase, consider defining a constant for the "judo.action.row-delete" translation key.You could add a constant at the top of the class:
private static final String ROW_DELETE_ACTION_KEY = "judo.action.row-delete";Then use it in the method:
} else if (ad.getIsRowDeleteAction()) { return ROW_DELETE_ACTION_KEY; }This approach would be consistent with best practices for maintainability and reduce the risk of typos in string literals.
369-369: LGTM! Consider refactoring for improved readability.The addition of the
row-deleteaction check is correct and consistent with the existing pattern. However, the long chain of boolean expressions might affect readability.Consider refactoring the
hasSystemTranslationmethod to improve readability. Here's a suggestion:
- Create an enum for action types:
private enum SystemAction { ADD, CLEAR, CREATE, BACK, CANCEL, SET, UPDATE, DELETE, ROW_DELETE, REMOVE, FILTER, FILTER_RELATION, REFRESH, REFRESH_RELATION, SELECTOR_RANGE, BULK_REMOVE, BULK_DELETE, OPEN_PAGE, OPEN_CREATE_FORM, OPEN_ADD_SELECTOR, OPEN_SET_SELECTOR, EXPORT }
- Create a method to check if an action is a system action:
private static boolean isSystemAction(ActionDefinition ad) { return Stream.of(SystemAction.values()) .anyMatch(action -> { try { String methodName = "getIs" + CaseFormat.UPPER_UNDERSCORE.to(CaseFormat.UPPER_CAMEL, action.name()) + "Action"; Method method = ActionDefinition.class.getMethod(methodName); return (boolean) method.invoke(ad); } catch (Exception e) { log.warning("Failed to check action: " + action); return false; } }); }
- Update the
hasSystemTranslationmethod:private static boolean hasSystemTranslation(VisualElement visualElement) { if (visualElement instanceof Button button) { ActionDefinition ad = button.getActionDefinition(); return !ad.getIsCustomAction() && isSystemAction(ad); } else if (visualElement instanceof ButtonGroup buttonGroup) { return buttonGroup.getFQName().endsWith("::PageActions"); } return false; }This refactoring would make the code more maintainable and easier to extend in the future.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (4)
- judo-ui-react/src/main/java/hu/blackbelt/judo/ui/generator/react/UiI18NHelper.java (2 hunks)
- judo-ui-react/src/main/resources/actor/public/i18n/system_default.json.hbs (1 hunks)
- judo-ui-react/src/main/resources/actor/public/i18n/system_en-US.json.hbs (1 hunks)
- judo-ui-react/src/main/resources/actor/public/i18n/system_hu-HU.json.hbs (1 hunks)
🔇 Additional comments (1)
judo-ui-react/src/main/java/hu/blackbelt/judo/ui/generator/react/UiI18NHelper.java (1)
Line range hint
1-386: Overall assessment: Changes are correct and consistent.The modifications to handle the new
row-deleteaction have been implemented correctly and consistently with the existing codebase. The changes enhance the translation capabilities of the UI framework by integrating additional action translations for buttons, specifically targeting therow-deletefunctionality.While the implementation is correct, there are opportunities for minor improvements:
- Consider using constants for translation keys to improve maintainability.
- The
hasSystemTranslationmethod could benefit from refactoring to enhance readability and extensibility.These suggestions aim to further improve code quality without changing the core functionality.
