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

OBPIH-5202 Not translatable with crowdin in Consumption report #3740

Merged
merged 2 commits into from
Jan 3, 2023

Conversation

kchelstowski
Copy link
Collaborator

No description provided.

@@ -20,64 +20,83 @@
<g:hiddenField name="parametersHash" value="${command?.generateParametersHash()}"/>

<div class="box">
<h2><warehouse:message code="consumption.parameters.label" default="Parameters"/></h2>
<h2><g:message code="consumption.parameters.label" default="Parameters"/></h2>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are we changing theses? I thought we agreed not to change the namespace.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I remember at some point you wanted to get rid of warehouse:message so having an occasion I decided to change it to g:message. If you would like to stick with warehouse:message for now, I can bring it back

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, I don't want to change anything with these taglibs in the develop branch because it adds more work for the rebase on the Grails 3 branch.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So revert all of those. If there's an issue with warehouse:message we can discuss that but please refrain from making small refactorings like this. It can add a lot of complexity to the rebase so I want to limit the changes we make to existing code.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, sure

@@ -516,6 +540,10 @@ default.success.message=Success
default.summary.label=Summary
default.support.label=Help
default.systemError.label=System error
default.dataTable.noData.label=No data available in table
default.dataTable.showingEntries.label=Showing _START_ to _END_ of _TOTAL_ entries
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I neglected to comment on this before. Why are we using this syntax instead of args i.e. {0} {1}? And where / when do these parameters get replaced?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are not the same args as {0}, {1}, these are specific datatable params that need to be provided like this and datatable "decodes" those params and substitute proper numbers into this.
Source:
https://datatables.net/examples/i18n/options.html

@@ -20,64 +20,83 @@
<g:hiddenField name="parametersHash" value="${command?.generateParametersHash()}"/>

<div class="box">
<h2><warehouse:message code="consumption.parameters.label" default="Parameters"/></h2>
<h2><g:message code="consumption.parameters.label" default="Parameters"/></h2>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, I don't want to change anything with these taglibs in the develop branch because it adds more work for the rebase on the Grails 3 branch.

@@ -20,64 +20,83 @@
<g:hiddenField name="parametersHash" value="${command?.generateParametersHash()}"/>

<div class="box">
<h2><warehouse:message code="consumption.parameters.label" default="Parameters"/></h2>
<h2><g:message code="consumption.parameters.label" default="Parameters"/></h2>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So revert all of those. If there's an issue with warehouse:message we can discuss that but please refrain from making small refactorings like this. It can add a lot of complexity to the rebase so I want to limit the changes we make to existing code.

@@ -26,7 +26,7 @@

<div class="box">
<h2>
${warehouse.message(code:'consumption.label')} <small>(${command.rows?.keySet()?.size()} results)</small>
${warehouse.message(code:'consumption.label')} <small>(${command.rows?.keySet()?.size()} ${g.message(code: 'default.resultsLowerCase.label', default: 'results')})</small>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the future, let's not add codes for lowercase vs uppercase. You should be able to the following

${g.message(code: 'default.resultsLowerCase.label', default: 'results').toLowerCase()}

or

<%@ page import="import org.apache.commons.lang3.StringUtils" %>
${StringUtils.lowerCase(g.message(code: 'default.results.label', default: 'Results'))}

@jmiranda jmiranda merged commit f99dc55 into develop Jan 3, 2023
@jmiranda jmiranda deleted the OBPIH-5202 branch January 3, 2023 17:17
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.

None yet

3 participants