i18n finished #261

sebastianapw · 2020-04-02T13:19:12Z

Hey there,
I've finished i18n of Osler. There should be a way of configuring the language on the first setup of Osler. For now the language will be configured in base_settings.py.

justinrporter · 2020-04-25T17:08:25Z

appointment/templates/appointment/form_submission.html

 {% load bootstrap3 %}

 {% block title %}
 {{ note_type }} {{ patient.name }}
 {% endblock %}

 {% block header %}
-<h1>New {{ note_type }} </h1>
+<h1>{% trans "New" context "neuer" %} {{ note_type }} </h1>


What's going on in this line? I'm surprised to see any non-base language words (in this case "neuer")... is that necessary?

These are context based translations. You are right, there should be a better way than to write it down in german! Basically this tells the i18n script that this word "new" can have different meanings or translations in german (e.g. it could translate to "neu" oder "neuer"). I try to think about a better way to implement this.

justinrporter · 2020-04-25T17:09:44Z

pttrack/fixtures/pttrack.json

@@ -1,4 +1,5 @@
 [
+{% load i18n %}


Does this work?! I would expect the json parser to choke on this. Does it actually respect the template language?

I left this one in the json file by mistake. It's wrong, you are right.

justinrporter · 2020-04-25T17:10:09Z

pttrack/models.py

@@ -45,7 +52,7 @@ def make_filepath(instance, filename):


 class ContactMethod(models.Model):
-    '''Simple text-contiaining class for storing the method of contacting a
+    '''Simple text-contsaining class for storing the method of contacting a


Should be "text-containing"

justinrporter · 2020-04-25T17:10:58Z

pttrack/models.py



    pcp_preferred_zip = models.CharField(max_length=5,
                                         validators=[validators.validate_zip],
                                         blank=True,
-                                         null=True)
+                                         null=True, verbose_name=_("pcp preferred zip"))


Does this create the need for a database migration? What happens when you run manage.py makemigrations?

justinrporter · 2020-04-25T17:12:18Z

pttrack/static/pttrack/css/bootstrap-theme.min.css

I'm pretty surprised you have changes here to the minified bootstrap code. Is this just your editor being a bit enthusiastic about formatting things?

justinrporter · 2020-04-25T17:13:25Z

pttrack/templates/pttrack/base.html

@@ -38,6 +40,7 @@
    body {
      padding-top: 50px;
      padding-bottom: 5%;
+      background-color: rgb(238, 242, 245)


Why the addition of a background color here?

I've played around a little with the design, this can be deleted and slipped into the commit by mistake

"ReferralStatus" -> "Referral Status"

justinrporter · 2020-04-25T17:18:46Z

pttrack/templates/pttrack/patient_detail.html

-							<p><a href="{% url 'followup' pk=note.pk model=note.type %}"><strong>{{ note.type }} Followup:</strong></a> {{ note.short_text }}</p>
-							<p class="text-muted text-right">by {{ note.author }} ({{ note.author_type }}) at {{ note.written_datetime }}</p>
+							<p><a href="{% url 'followup' pk=note.pk model=note.type %}"><strong>{{ note.type }} {% trans 'Followup' %}:</strong></a> {{ note.short_text }}</p>
+							<p class="text-muted text-right">{% trans 'by' %} {{ note.author }} ({{ note.author_type }}) {% trans 'at' %} {{ note.written_datetime }}</p>


When we have a {% trans 'by' %} do we ever have to worry about the word "by" translating to different things? Like a note by me is "von mir" but if you say you're standing by me you stand "neben mir". See what I mean?

Yes, that's right! As I've mentioned in another comment there is a "context"-option, that helps to solve this problem. This should be used whenever there are words with multiple translations.

justinrporter

Let's discuss the need for changes to the boostrap css
I want to make sure we're not affecting database structure with the translation
We may need to look into contextual markers for some things, such as the use of the word "by" for authorship.
The appearance of  after the loading of the i18n tags seems like it has the potential to confuse future developers.
There are several places where {% blocktrans %}{% endblocktrans %} directives are used where {% trans 'variable' %} directives seem like they'd be more concise.
A few miscellaneous things.

justinrporter · 2020-04-25T17:20:44Z

referral/forms.py



 class FollowupRequestForm(ModelForm):
    class Meta:
        model = models.FollowupRequest
        fields = ['due_date', 'contact_instructions']
        widgets = {'due_date': DateTimePicker(
-            options={"format": "MM/DD/YYYY"})}
+            options={"format": _("MM/DD/YYYY")})}


When you translate this, do you change it to European style ("DD/MM/YYYY")? If so, internal logic will likely need to change.

justinrporter · 2020-04-25T17:23:58Z

referral/models.py

-        (STATUS_UNSUCCESSFUL, 'Unsuccessful'),
+        (STATUS_SUCCESSFUL, _('Successful')),
+        (STATUS_PENDING, _('Pending')),
+        (STATUS_UNSUCCESSFUL, _('Unsuccessful')),


Does this change the database? (I don't think so, I think that's what STATUS_PENDING does, but I'm not sure.)

justinrporter · 2020-04-25T17:24:52Z

referral/templates/referral/new-FQHC-referral.html

@@ -1,13 +1,16 @@
 {% extends "pttrack/base.html" %}

+{% load i18n %}
+<!-- trans finished -->


What's the purpose of this comment?

This can be deleted, it was just so that I'm sure that I've already finished the translation of that particular file.

justinrporter · 2020-04-25T17:28:40Z

referral/templates/referral/new-patient-contact.html

 {% endif %}
-<p class="lead">Referral to 
+<p class="lead">{% blocktrans %}Referral to{% endblocktrans %}


I think the blocktrans directive supports putting django template language stuff inside it, so you could have this be all one big blocktrans, no?

justinrporter · 2020-04-25T17:30:43Z

referral/templates/referral/new-patient-contact.html

@@ -28,7 +31,7 @@ <h1>New referral followup</h1>
 <div class="container">
 	<div class="row">
 		<div class='col-md-offset-2 col-md-8' style='padding-bottom: 2em'>
-		  <p>A referral is considered <strong><span class='text-success'>successful</span> only once a patient has both made <i>and gone to</i> and appointment</strong>. Please <strong><span class='text-info'>continue following up</span></strong> with patients until they have completed an appointment. Only once you have exhausted followup should you save as an <strong><span class="text-danger">unsuccessful referral.</span></strong></p>
+		  <p>{% blocktrans %}A referral is considered{% endblocktrans %} <strong><span class='text-success'>{% blocktrans %}successful{% endblocktrans %}</span> {% blocktrans %}only once a patient has both made{% endblocktrans %} <i>{% blocktrans %}and gone to{% endblocktrans %}</i> {% blocktrans %}and appointment{% endblocktrans %}</strong>. {% blocktrans %}Please{% endblocktrans %} <strong><span class='text-info'>{% blocktrans %}continue following up{% endblocktrans %}</span></strong> {% blocktrans %}with patients until they have completed an appointment{% endblocktrans %}. {% blocktrans %}Only once you have exhausted followup should you save as an{% endblocktrans %} <strong><span class="text-danger">{% blocktrans %}unsuccessful referral{% endblocktrans %}.</span></strong></p>


This is quite verbose--there are a lot of blocktranss in here... is there really no way to have it just be one block?

I have to try that one out. But as far as I remember the html-tags inside the blocktrans tags are slipping into the translation, this can be confusing and could lead to errors while translating (e.g. not closing the tag in the right manner).

justinrporter · 2020-04-25T17:32:36Z

referral/templates/referral/select-referral-type.html

 {% endblock %}

 {% block content %}

 <div class="container text-center">
 	<div class="btn-group-vertical">
  	{% for referral_type in referral_types %}
-	    <a class="btn btn-{% cycle 'info' 'success' 'warning' 'danger' %}" href="{% url 'new-referral' rtype=referral_type.slugify pt_id=pt.pk %}" role="button">Create {{referral_type}} referral</a>
+	    <a class="btn btn-{% cycle 'info' 'success' 'warning' 'danger' %}" href="{% url 'new-referral' rtype=referral_type.slugify pt_id=pt.pk %}" role="button">{% blocktrans %}Create{% endblocktrans %} {{referral_type}} {% blocktrans %}referral{% endblocktrans %}</a>


If I've understood the blocktrans docs correctly, you can do:

{% blocktrans %}Create {{referral_type}} referral{% endblocktrans %}

instead of what you have here.

You are right, didn't catch that one

justinrporter · 2020-04-25T17:33:06Z

requirements.txt

@@ -2,6 +2,7 @@ Django==1.11.23
 django-bootstrap3==11.0.0
 git+https://github.com/inducer/django-bootstrap3-datetimepicker.git
 django-crispy-forms==1.7.2
+django-debug-toolbar==1.11


How did this get in here?

First time I started Osler in a Virtual Environment it threw an error that said missing django-debug-toolbar. I've added it in requirements.txt, probably I've didn't use the developer mode.

justinrporter · 2020-04-25T17:35:19Z

workup/templates/workup/new-note-dispatch.html

+	<div  class="col-md-6 col-md-offset-3 custom-table-outer">
+		<table class="table custom-table" style="text-align: center">
+			<tr><th class="custom-table-inner" style="text-align: center"><h4 class="h4-custom">{% trans 'Note Type' %}</h4>
+			<p class="p-custom">Wähle hier die Untersuchungsform aus, die für diesen Patienten angelegt werden soll.</p>


Was geht denn heir ab? Falsche sprache?

Yes, wrong language. This can be deleted, I've added it for explaining what "Note type" means (students who use the Osler for the first time may have no idea what is meant by "Note type").

justinrporter · 2020-04-25T17:37:32Z

workup/templates/workup/workup_body.html

        </div>
    </div>

    <div class="row">
-        <h4>Physical Exam</h4>
+        <h4>{% blocktrans %}Physical Exam{% endblocktrans %}</h4>


Many of these would be more concisely represented with as {% trans 'string' %} blocks.

I've also thought about that. Especially when using long phrases and when inserting special characters like apostrophes using {% trans 'string' %} is throwing errors.

seb-ssabes added 3 commits March 30, 2020 19:20

do most of the i18n

d5bddd2

i18n finished

7662a3a

finished i18n

71f409a

justinrporter reviewed Apr 25, 2020

View reviewed changes

Update patient_detail.html

dea532d

"ReferralStatus" -> "Referral Status"

justinrporter reviewed Apr 25, 2020

View reviewed changes

justinrporter suggested changes Apr 25, 2020

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

i18n finished #261

i18n finished #261

sebastianapw commented Apr 2, 2020

justinrporter Apr 25, 2020

sebastianapw May 26, 2020

justinrporter Apr 25, 2020

sebastianapw May 26, 2020

justinrporter Apr 25, 2020

justinrporter Apr 25, 2020

justinrporter Apr 25, 2020

justinrporter Apr 25, 2020

sebastianapw May 26, 2020

justinrporter Apr 25, 2020 •

edited

Loading

sebastianapw May 26, 2020

justinrporter left a comment

justinrporter Apr 25, 2020

justinrporter Apr 25, 2020

justinrporter Apr 25, 2020

sebastianapw May 26, 2020

justinrporter Apr 25, 2020

justinrporter Apr 25, 2020

sebastianapw May 26, 2020

justinrporter Apr 25, 2020

sebastianapw May 26, 2020

justinrporter Apr 25, 2020

sebastianapw May 26, 2020

justinrporter Apr 25, 2020

sebastianapw May 26, 2020

justinrporter Apr 25, 2020

sebastianapw May 26, 2020

i18n finished #261

Are you sure you want to change the base?

i18n finished #261

Conversation

sebastianapw commented Apr 2, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

justinrporter Apr 25, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

justinrporter left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

justinrporter Apr 25, 2020 •

edited

Loading