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

Additional strings #8433

Merged
merged 7 commits into from Sep 15, 2021
Merged

Conversation

marcellamaki
Copy link
Member

Adds missing strings, fixes typos, removes/streamlines duplicates.

@radinamatic
Copy link
Member

Can you change the message for noUsersExistLabel into 'There are no users in this facility'?

context:
'Option that allows the user to prevent this resource from displaying in the future while using category search',
},
markResourceAsCompleteLabel: {
Copy link
Member

Choose a reason for hiding this comment

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

This can also be reused in the MarkAsCompleteModal.

Also, the Resource completed string appears in both MarkAsCompleteModal and the CompletionModal. Can you move it in the commonLearn strings and reuse?

@radinamatic
Copy link
Member

Can you change the message in the markResourceAsCompleteConfirmation to be 'Are you sure you want to mark this resource as completed?' (completed instead of finished).

@radinamatic
Copy link
Member

radinamatic commented Sep 15, 2021

And as mentioned in Slack, since this value needs to be pluralized, change the message for results into:

message: '{results, number, integer} {results, plural, one {result} other {results}}',

@radinamatic
Copy link
Member

Please change the value for the browseChannel to be 'Browse channels' (sentence case).

@radinamatic
Copy link
Member

We have LearningActivities defined in both commonCore and commonLearn strings 🤔

LEDev2104 (start)  Running  - Oracle VM VirtualBox_034

I see those from commonCore used only in the LearningActivityLabel component (which is also in the Learn plugin 🙂), so maybe it makes sense to apply those from the commonLearn, and delete the definitions in the commonCore?

@rtibbles
Copy link
Member

I see those from commonCore used only in the LearningActivityLabel component (which is also in the Learn plugin slightly_smiling_face), so maybe it makes sense to apply those from the commonLearn, and delete the definitions in the commonCore?

Given that we will be almost certainly using this metadata in Coach and Device at some point in the future, it makes the most sense to move all these strings into commonCore.

@marcellamaki
Copy link
Member Author

Okay - since the version in learn is more complete, I will copy that over to core to retain the context, but delete from learn.

@radinamatic
Copy link
Member

Delete the line 474 from the commonCore strings, as that category (Logic and critical thinking) is duplicated, and defined also in the line 497.

@radinamatic
Copy link
Member

Still pending to refactor the Explore resources to... strings.

If I remember correctly, we decided to concatenate like so: Explore resources - '{ category }' cc @rtibbles

@radinamatic
Copy link
Member

Can you change the message in the examReportTitle to be 'Report for { examTitle }' (for clarity)?

@marcellamaki
Copy link
Member Author

Still pending to refactor the Explore resources to... strings.

If I remember correctly, we decided to concatenate like so: Explore resources - '{ category }' cc @rtibbles

yes - this is in the code I have not pushed yet. Trying to consolidate so you are not re-reviewing too many times!

@radinamatic
Copy link
Member

This Accessibility string can be reused from the commonCore strings.

And while you're at it, I also see the string Language in the following components:

  • SelectGroup
  • SidePanelResourceMetadata
  • FilteredChannelListContainer
  • MediaPlayerIndex

While Level is repeated in 2 components:

  • SelectGroup
  • SidePanelResourceMetadata

High time to put both into commonCore 😉

Copy link
Member

@radinamatic radinamatic left a comment

Choose a reason for hiding this comment

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

Final push looking good! 👏🏽 💪🏽

@rtibbles rtibbles merged commit d9e8e70 into learningequality:develop Sep 15, 2021
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.

None yet

3 participants