-
Notifications
You must be signed in to change notification settings - Fork 632
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
Add strings for post-setup onboarding guide component #12113
Add strings for post-setup onboarding guide component #12113
Conversation
Build Artifacts
|
@@ -0,0 +1,63 @@ | |||
import { createTranslator } from 'kolibri.utils.i18n'; | |||
|
|||
export const kolibriOnboardingGuideStrings = createTranslator('kolibriOnboardingGuideStrings', { |
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.
I guess my only question here, is that all these strings seem to be specific to the learn experience, so why do we need to have them in kolibri-common
and not in the learn app specifically?
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.
Good point. This is apart of the new method we’re trying out recommended to me by Marcella, putting all of the strings into one file per feature. Similar to how the enhancedQuizManagementStrings
are specific to the coach plugin, I followed that setup. However, I don’t think this file necessarily has to be within the kolibri-common
folder, it is just a convenient place to easily locate all of the new strings. @marcellamaki can probably correct me on this or add more context.
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.
I've thought a few times about moving the enhancedQuizManagementStrings
into Coach. I'm keen on the idea sort of treating features as modules.
For example, in my case, "QuizCreation" could be it's own directory with it's relevant feature-specific state-management modules, strings files, constants, etc. I'm not super confident I'm fully considering possible downsides to this though.
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.
Yeah that's a good question, @rtibbles and I don't know actually if we made a concrete decision about where these feature files belong. Looking back through slack it seems like we only agreed that it would be one file per feature and namespaced, and I suggested to Liana that she follow the EQM model and did not give it much additional thought.
It does seem like for features that only impact one plugin, that the strings live in that plugin, but still in a strings folder with the feature namespace. And then, maybe when we have features that are cross-plugin, we can put those into kolibri-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.
maybe the strings that are only used in one plugin could live in [plugin]/src/strings/namespacedFeatureStringFile.js
(with /strings/ being a new folder that we would need to create.
What do you think @rtibbles @nucleogenesis @LianaHarris360 ?
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.
I hadn't realised that plugin specific strings were being put into kolibri-common at all, so yes, I think putting strings that are only used in one plugin in that plugin makes a lot more sense (both in this case and for the enhanced quiz management strings).
It was never the intention of kolibri-common to become a place where all new things get put, only for things that are explicitly used across multiple plugins.
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.
For EQM I had thought there were new strings that were used in both learn and coach, but actually there might not be any in learn, or if there are a few, I guess it's likely that they are not actually shared (i.e. the same string used in both places). So this would be good to check. @nucleogenesis let's open an issue for this and get it sorted before we get to string freeze?
@LianaHarris360 - so the next step would just be moving this file into Learn, and then I think it should be good to merge.
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.
Overall the strings definitions look good. I double checked against Figma. I won't approve as I think it's worth resolving the location discussion first, but beside that this LGTM
@@ -0,0 +1,63 @@ | |||
import { createTranslator } from 'kolibri.utils.i18n'; | |||
|
|||
export const kolibriOnboardingGuideStrings = createTranslator('kolibriOnboardingGuideStrings', { |
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.
I've thought a few times about moving the enhancedQuizManagementStrings
into Coach. I'm keen on the idea sort of treating features as modules.
For example, in my case, "QuizCreation" could be it's own directory with it's relevant feature-specific state-management modules, strings files, constants, etc. I'm not super confident I'm fully considering possible downsides to this though.
}, | ||
noSourceConnectionTextDescription: { | ||
message: | ||
'No channels yet. Start exploring libraries around you and find materials to add to your library.', |
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.
If this is the correct frame in Figma, the string should say:
There is nothing in your library yet. Explore libraries around you and start adding materials to your own.
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 string is for this frame, I did not find it already existing in Kolibri.
We will do a final user strings review once the feature has been implemented, just noting that in this pre-existing string we should keep the terminology consistent and use the word
|
Summary
This pr adds the reviewed strings for the Post-Setup Onboarding Guide component work to Kolibri.
Newly Added Strings:
Pre-existing Strings:
References
Closes #12102
Reviewer guidance
packages/kolibri-common/strings/kolibriOnboardingGuideStrings.js
PR process
Reviewer checklist
yarn
andpip
)