string cleanup and refactoring

[indirectlylit]

Observed behavior

0.12.0 has quite a bit of string-related tech debt:

• crossComponentTranslator was used/abused to allow components to use translated strings from other components
• A subset of strings is now in a 'common coach strings' file
• A subset of strings now uses more precise naming conventions to distinguish different parts of speech - for example previewLabel (a noun) vs previewAction (a verb)
• Many strings defined are no longer used

Expected behavior

• We should add four more shared string files: one for core, and one for each of the plugins (learn, facility, device)
• We should remove all uses of crossComponentTranslator by either moving the string to the new component or moving the string to a shared file
• We should move common strings to the new shared string files and give them explicit, conventional names that reflect their part of speech.
• We should create a new linting rule that warns about unused strings

Note on common strings:

First, we should remove the use of computed props and mixins. Instead, we should use $formatMessage(namespace, stringID). See discussion

Second, we should be judicious about what strings are put in 'common' files because over-doing this can have adverse affects. In particular, sometimes strings that are the same in English actually need to be translated differently in other languages. For example:

• "preview" the verb vs "preview" the noun
• adjectives that need to agree with the thing they're modifying
• words that are context-specific, and just happen to be the same in English

Therefore, we should not put a string in common unless it is:

1. used at least 3 times
2. used in exactly the same way (label, button, etc)
3. used to mean exactly the same thing. All strings added to common files should follow the naming conventions in commonCoachStrings

User-facing consequences

none, but affects the costs and difficulty for devs and transators

Context

0.12.0

[jonboiser]

I would add too that the amount of extraneous data in the DevTools has gone up a lot, making them harder to read for me

1. Extra computed items within Components coming from mixins
2. About 20-30 getters for kolibriCoreTheme, one for almost each color and some styles
3. The SET_MODALITY mutation appears in the Vuex feed on every blur (e.g. just clicking a button).

I would like to see some effort in reducing the amount of extra data in devtools.

[indirectlylit]

yeah, it's true. Do $tr strings show up in devtools though?

[rtibbles]

We could use the new Vue observable method to manage some of the watched state (like the modality) without having to put it in the store. Could do a similar refactor to the dynamic styles as well.

I think the issue with the strings is that the common strings are exposed as a huge computed prop, so that also inflates things.

[indirectlylit]

I think the issue with the strings is that the common strings are exposed as a huge computed prop, so that also inflates things.

oh right - because it's a mixin.

I see there's a feature request to have devtools be smarter about vuex module organization so if we keep leveraging modules, things may improve on their end also ... eventually: vuejs/devtools-v6#327

[jonboiser]

Some codes that might be able to be deleted:

1. LearnerExerciseDetailPage and its dependencies LearnerExerciseReportOld , AttemptSummary
2. ExamReportPage and ManageExamModals, after we extract the cross-component-translator strings from them

[nucleogenesis]

@indirectlylit @rtibbles @jonboiser

What I've Done
The approach I decided to take was to first get an overview of all of the $tr() usage in the parts of Kolibri mentioned in the issue: Core, Learn, Device, Facility, Coach so that I can easily see which strings need to be extracted to a common file.

You can see the results of the profiling here.

Note that there are 5 sheets in that doc at the bottom - each sheet is a profile of the section it's named by.

See the Profiler code here.

Thoughts
It seems to me that there are a handful of $tr keys that can be extracted to a common file, but there won't be many for each.

As for Coach, there seems to be many some which are used in many places, but are not defined in commonCoachStrings (example: allQuizzes).

In all cases, this should be a solid guide for me to do the qualitative work regarding the criteria that Devon proposed - that is - making sure that the text is used consistently enough to deserve moving to a common string file.

Next Actions

• Discuss with all stakeholders if I should proceed based on the criteria described in the original issue.
• Work out a way to find defined $trs that are not used at all.
• Further review that my profiles are accurate.

From there - I can proceed with refactoring according to the discussion Richard and Devon had in Slack regarding moving away from the mixins approach and using $formatMessage with the appropriate namespace.

[rtibbles]

My impression from looking at your profiling code that it is matching based on key, rather than string content? This might be an issue as it may give false positives of repeated strings.

For any future tooling, might be good to use the existing message extraction code that we have - happy to give insights into extending/modifying it for the needs outlined above.

[indirectlylit]

this work is done I believe!

@nucleogenesis correct me if I'm wrong