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.

Sign up for GitHub

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

Jump to bottom

feat(Multiple Languages): Show ProjectLanguageChooser in AT #1550

Merged
merged 5 commits into from
Dec 12, 2023
Merged

feat(Multiple Languages): Show ProjectLanguageChooser in AT #1550

merged 5 commits into from
Dec 12, 2023

Conversation

hirokiterashima
Copy link
Member

@hirokiterashima hirokiterashima commented Dec 5, 2023 •
edited
Loading

Note

  • Don't worry about styles in this PR
  • Remember to use the [field].i18n = { id: "abc123", "modified": 123 } project format for this PR and all future PR's.

Changes

  • Change ProjectLanguageChooserComponent so we can use it in the VLE and the AT
    • Take in ProjectLocale as @Input
    • Emit selected Language as @Output
    • Move to common folder
  • Display ProjectLanguageChooserComponent in the AT's TopBarComponent if unit has translations
    • Update available language options when author changes them in the Project Info view

Test AT

  • ProjectLanguageChooserComponent displays supported languages iff unit has translations
  • Editing language options in the Project Info view updates the ProjectLanguageChooserComponent's available options
    • If there are no available languages, the ProjectLanguageChooserComponent should not appear

*Note: nothing should happen we switch the language option in the ProjectLanguageChooserComponent atm. We'll handle that in a future PR.

Test Student VLE

  • Switching between translations works as before. While logged in as a student or previewing a unit, test that:
    • Languages drop-down appears only if the unit has at least 1 supported locale
    • Switching between the language options immediately translates elements of the page that have translations
      • Test everywhere: unit plan, step select drop down, unit title
      • Elements that don't have translations appear in default locale
      • You can switch back to default locale

@hirokiterashima
Change ProjectLanguageChooser to take in ProjectLocale as input and
e02cd79 
emit selected Language as output. This will let us reuse this component
in the AT.
@hirokiterashima
Move ProjectLanguageChooser to common folder and show it in the AT's
b1d855b 
TopBarComponent. Update options when author changes them in the Project
info view
@hirokiterashima hirokiterashima added the enhancement New feature of any size or improvement (UI, performance, security) label Dec 5, 2023
@hirokiterashima hirokiterashima self-assigned this Dec 5, 2023
@hirokiterashima hirokiterashima marked this pull request as ready for review December 5, 2023 23:30
geoffreykwan
geoffreykwan requested changes Dec 6, 2023
View reviewed changes
Copy link
Member

@geoffreykwan geoffreykwan left a comment

Choose a reason for hiding this comment

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

Mostly works but I found one issue.

In the Authoring Tool, if you have set the language in the top bar to one of the additional languages and then go to the Unit Info view and change the default language or add/delete an additional language, the language in the top bar changes back to the default language.

I think how we should handle this is if the author is in the Unit Info view and changes the default language or additional languages, the top bar language should not change. The top bar language should only change to the default language if the language that is currently selected in the top bar is removed from the additional languages.

@hirokiterashima
Set selected language to default language only when previously-selected
45bac6a 
option is no longer available (i.e., removed by author), or this is the
first time the component is displayed
@hirokiterashima
Copy link
Member Author

@geoffreykwan Good catch. We now set selected language to the default language only when the previously-selected option is no longer available (i.e., removed by author), or this is the first time that the component is displayed. PTAL.

geoffreykwan
geoffreykwan approved these changes Dec 7, 2023
View reviewed changes
Copy link
Member

@geoffreykwan geoffreykwan left a comment

Choose a reason for hiding this comment

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

Looks good. I have some minor suggestions.

src/app/common/project-language-chooser/project-language-chooser.component.ts Outdated Show resolved Hide resolved
src/app/common/project-language-chooser/project-language-chooser.component.spec.ts Outdated Show resolved Hide resolved
src/app/common/project-language-chooser/project-language-chooser.component.spec.ts Outdated Show resolved Hide resolved
src/app/common/project-language-chooser/project-language-chooser.component.spec.ts
setProjectLocale(new ProjectLocale({ default: 'en_US', supported: ['ja', 'es'] }));
});

it('shows available languages and selects the default language', async () => {
Copy link
Member

Choose a reason for hiding this comment

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

Should this be wrapped in a describe that describes the initial state like describe('when the language chooser is first loaded') or something.

src/app/common/project-language-chooser/project-language-chooser.component.spec.ts
expect(await options[0].isSelected()).toBeTrue();
});

it('keeps selected language option when language option changes', async () => {
Copy link
Member

Choose a reason for hiding this comment

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

Should this be wrapped in a describe that describes the action like describe('when the language options change') or something.

@hirokiterashima hirokiterashima merged commit 8b22cc2 into issue-1513-multiple-languages-per-unit Dec 12, 2023
@hirokiterashima hirokiterashima deleted the multiple-languages-at-show-language-chooser-drop-down branch December 12, 2023 19:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@geoffreykwan geoffreykwan geoffreykwan approved these changes

Assignees

@hirokiterashima hirokiterashima

Labels
enhancement New feature of any size or improvement (UI, performance, security)
Projects
WISE Development 2023-2024
Status: Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@hirokiterashima @geoffreykwan