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

feat: Translate observation categories & details #458

Merged
merged 7 commits into from
Sep 24, 2020

Conversation

gmaclennan
Copy link
Member

@gmaclennan gmaclennan commented Sep 8, 2020

This PR adds functionality to Mapeo Mobile to display translated fields for presets. It depends on translations being built with mapeo-settings-builder v3.1.0 or later (presets built with earlier versions should still work, they will just not be translated).

mapeo-settings-builder@^3.1.0 includes scripts to extract messages from preset and field definitions for translation, and also a build script to include translated messages into the config package.

This PR should also fix the display of select fields, so that the label rather than the value is shown.

TODO:

  • Translate fields in share message

This PR is built on #448

Download a QA version of Mapeo for this PR: https://app.bitrise.io/artifact/48898947/p/6fe9bad13146a7fda75afc887d5a191a

Closes #455

@okdistribute
Copy link
Contributor

okdistribute commented Sep 10, 2020

Does this also need to be ported to Desktop? We didn't talk about that extra work today, not sure if it's needed?

@gmaclennan
Copy link
Member Author

Does this also need to be ported to Desktop? We didn't talk about that extra work today, not sure if it's needed?

It does, eventually. I do not think it is an immediate priority for @arky to also have this on Desktop. However it does also fix the display of select fields. As part of this PR I am abstracting the code to do this into re-usable components, which could be shared between Mobile and Desktop.

@@ -0,0 +1,154 @@
// @flow
Copy link
Contributor

@okdistribute okdistribute Sep 14, 2020

Choose a reason for hiding this comment

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

This seems useful! Can we abstract this out as part of this PR, perhaps to @mapeo/react-components? It would be great to re-use this in Md (and get the formatted preset names, too) for the Report. Come to think of it, the Md Report and the Mm Share screen ought to probably share a lot of code... :)

Copy link
Contributor

@okdistribute okdistribute Sep 17, 2020

Choose a reason for hiding this comment

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

I started this here. digidem/mapeo-components#1

In the desktop app I copy/pasted the relevant two components (FormattedFieldProp & FormattedValue) to get the Report PDF branch ready to launch. It would be an extra piece of work (1-2 weeks) to refactor these components into a shared library.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, wrote this in this way as a first step towards extracting into a shared library, see #460 however this depends on #459 which I would rather not do as part of this PR.

* meaningful translated values for null and boolean, but these types are not
* used widely in presets yet
*/
export function convertSelectOptionsToLabeled(
Copy link
Contributor

Choose a reason for hiding this comment

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

This also seems like it will be important to re-use this same rendering logic in other UIs, but it is not a react component. If the only argument is a SelectOptions object, I wonder how we can make it more tightly coupled, so it isn't in a 'utils' file?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah I have been wondering about that. This is used by:

  1. Select One data editing component
  2. Select Multiple data editing component
  3. Select one and multiple data view component

On desktop the editing component will likely be different (due to different UI patterns on Desktop) but will also need this function. As such I think it needs to be a helper function shared by these components. I think we can drop support for this in a later version of Mapeo Presets, or perhaps do this transform on the server. This is necessary because we originally supported options to be an array of strings, but later supported labels and values. Once we have a preset editor, these simplicity of just adding an array of strings is no longer so important.

// Look up label from field options. This is not necessary for presets
// created with mapeo-settings-builder@^3.1.0, which will have these options
// in the translation file, but is needed for older versions of presets
const options = convertSelectOptionsToLabeled(field.options);
Copy link
Contributor

@okdistribute okdistribute Sep 14, 2020

Choose a reason for hiding this comment

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

is there ever a time someone would use field.options in a Mapeo React component in the presentation/view layer without having it already be parsed by the 'convertSelectOptionsToLabeled' function? (not counting mapfilter hooks or custom functions that require the original field value)

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't think so, so perhaps we can transform options on the server or at the client api level, and treat Array<string> select options as a deprecated option that we change before the front-end receives it.

@gmaclennan gmaclennan changed the title feat: preset translations [WIP] feat: Translate observation categories & details Sep 24, 2020
@gmaclennan gmaclennan merged commit 044a8a0 into develop Sep 24, 2020
@gmaclennan gmaclennan deleted the feat/preset-translations branch September 24, 2020 17:27
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.

Displays "value", not "label" under details of observation
2 participants