-
-
Notifications
You must be signed in to change notification settings - Fork 2.2k
Fixes #278: Localization for Medication module #318
Fixes #278: Localization for Medication module #318
Conversation
@alexpelan unless I am missing something the medication module doesn't display searchStatus anywhere. The only place I see it being displayed is on the invoices index page. |
@jkleinsc Sorry if I was being unclear - 'searchStatus' appears (to me) to be used to filter which medication records show up - it looks like it searches for the model's 'status' property. The model's status property appears to be an english string, and is then displayed in the UI, for example in the medication/index template. These localization tickets appear to specifically target the UI text, I was just pointing out there's some other work that will have to be done. |
saved_message: 'The medication record has been saved.', | ||
fulfilled_title: 'Medication Request Fulfilled' | ||
}, | ||
common: { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@alexpelan was this common section intended to be labels common across the app? It is nested under medication in this file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, if it is intended to be app wide, a label other than "common" would be better.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It wasn't intended to be nested - great catch, thanks. What would you suggest as a better label? I was looking for something that could be a button, or a label, or a title
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@alexpelan I think it would be better to place them in more specific buckets such as button, label, or title because it gives translators better context of the intention of that text. Does that make sense?
@alexpelan gotcha about searchStatus. Yes the problem is the text in the model. |
modelName: 'medication', | ||
newTitle: 'New Medication Request', | ||
newTitle: t('medications.titles.new_medication_request'), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be t('medication.titles.new_medication_request'),
2ce43bd
to
27b36ea
Compare
@jkleinsc responded to your feedback - thanks for the quick review. |
@alexpelan looks good. Thanks for the PR! |
Fixes #278: Localization for Medication module
One thing I wanted to highlight is the 'searchStatus' parameter used in many of the index routes isn't easy to internationalize right now - because that same string is used for both display and is actually stored in the model - I think we'll have to switch to use category id: string mappings and translate those for the models?