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

Workflows Internationalization #3634

Open
wants to merge 5 commits into
base: dev
from

Conversation

Projects
None yet
3 participants
@monoludic
Copy link
Contributor

commented May 14, 2019

There's a a nested call to T[]. You'll tell me if it's OK.

monoludic added some commits Apr 30, 2019

Badge "disabled" in PascalCase
consistency with WorkflowStatus
@dnfclas

This comment has been minimized.

Copy link

commented May 14, 2019

CLA assistant check
Thank you for your submission, we really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.

monoludic sign now
You have signed the CLA already but the status is still pending? Let us recheck it.

@@ -19,7 +19,7 @@
<a class="nav-link active" id="workflow-tab" data-toggle="pill" href="#workflow" role="tab" aria-controls="workflow" aria-selected="true">Workflow</a>
</li>
<li class="nav-item">
<a class="nav-link" id="state-tab" data-toggle="pill" href="#state" role="tab" aria-controls="state">State</a>
<a class="nav-link" id="state-tab" data-toggle="pill" href="#state" role="tab" aria-controls="state">@T["State"]</a>

This comment has been minimized.

Copy link
@sebastienros

sebastienros May 14, 2019

Member

Good catch

@@ -76,7 +76,7 @@
<div>
<span class="hint">@T["Created {0}", (object)(await DisplayAsync(await New.TimeSpan(Utc: entry.Workflow.CreatedUtc)))]</span>
<div class="info">
<span class="badge badge-@statusCss">@entry.Workflow.Status</span>
<span class="badge badge-@statusCss">@T[entry.Workflow.Status]</span>

This comment has been minimized.

Copy link
@sebastienros

sebastienros May 14, 2019

Member

Enums should be localized using a switch statement that resolve to calls to T[] with the static string inside it. This way our tool will be able to pick it up and add the entry in pot files.

@@ -1,7 +1,7 @@
@model OrchardCore.Workflows.ViewModels.ActivityEditViewModel

<div data-workflow-type-id="@Model.WorkflowTypeId" data-activity-id="@Model.ActivityId">
<h1>@RenderTitleSegments(T["Edit {0}", Model.Activity.Name.CamelFriendly()])</h1>
<h1>@RenderTitleSegments(T["Edit {0}", T[Model.Activity.Name.CamelFriendly()]])</h1>

This comment has been minimized.

Copy link
@sebastienros

sebastienros May 14, 2019

Member

This one is tricky. It's not a dynamic value (a class) and it can't be solved with a switch.
I think in this case we could create a new shape that would use the activity name in the alternate. Or add a new property on the class to return a localized string.
Another option would be to have a custom file to add new pot entries with their context, such that each module could define the activity names, and your suggestion would then work fine.

This comment has been minimized.

Copy link
@sebastienros

sebastienros May 14, 2019

Member

I think adding a new property on the activity is better. Like DisplayText

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.