-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
Frontend multi-language support #1690 #1790
Conversation
Very cool, thanks for the PR!! I won't have any bandwidth to review PRs this week as we've got some live streams happening, but I'll take a look next week. |
@microsoft-github-policy-service agree company="NEC Corporation" |
Please review this PR as it is nearly complete. I will make any necessary revisions based on your feedback. Thank you. |
Okay, I will review this week, thanks for letting me know it's ready! |
@bnodir This is so cool! I pushed one change to move the dropdown to a fluentUI dropdown and add a label, which makes the UI more consistent and fixes an accessibility warning. We need to make the language picker optional, however, as not all developers will be able/willing to support translating all their strings, and some developers are targeting a single language. I think the way to do that is the same way we decide whether to show the vision options- modify the /config route to pass down a "showLanguagePicker" option, add an azd environment variable like ENABLE_LANGUAGE_PICKER, and set that accordingly. Also, if you can, please compare the build times before this change and after this change. We always want to do that when adding new npm packages, so that we're aware of the performance hit of adding a particular package. |
@zedhaque I think you were looking into i18n, potentially, for your audience? Am curious if you have any feedback on this PR. Also if you foresee issues with responsiveness. |
@pamelafox Thank you for reviewing the PR and providing your suggestions! I will work on your feedback this week and let you know once I am finished. Meanwhile, could I kindly ask you to have a native speaker review the translations, as they were generated by AI? Regarding making the language picker optional and comparing build times, I will work on implementing the approach you suggested and may reach out if help is needed. |
@bnodir Sure, I will get a native reviewer for translations, thanks for letting me know they were AI generated. |
@bnodir - just tried your branch. great work thank you! A quick request - I could use the language value in the API payload. If possible. |
One observation - When I enabled "Speech Out" on the chat response page and pressed the "sound" button, the text-to-speech function works depending on the answer language. However, if I went back, changed to another language, and tried again, the speech output remained as the last spoken one. The text-to-speech output remains cached in the previous language after changing the language setting. It only updates to the new language after continuing with further chat interactions. |
@zedhaque - Thank you for your feedback and kind words. Regarding your request to include the language value in the API payload, could you please clarify it a little more? Is this intended to be used as a value for the search query, instead of the configuration instructions provided below?
As for your observation, I checked it several times but was unable to reproduce it on my side. Could you please provide more details or steps to help me understand the issue better? |
@bnodir - I haven't had a chance to try your latest changes yet. I will test the latest repo and get back to you regarding the "speech out" observation. Regarding the API payload, I was referring to passing the language value that the user selects via overrides (see the attached screenshot). You can see how other values from the developer settings are being populated. This would allow me to use the selected language value in the backend, as it will be available via overrides. Many thanks for looking into this. |
@zedhaque - Thank you for the clarification. Could you please check the attached screenshot to confirm whether the payload request looks correct? If you have any specific adjustments or suggestions, please let me know. |
@bnodir - many thanks 🙏 yes exactly what I wanted. I come back to you tomorrow on the speech out item. |
@bnodir - Confirming that the issue with "speech out" is not related to your code. It occurs both in the latest repo and in your repo. @john0isaac - FYI as you are working on #1894 - please see the attached video. The issues are:
Let me know if you'd like me to create a bug report. speech-out.mp4 |
@zedhaque I did say that they are out of the scope of the PR but went ahead and fixed all of them anyway. |
Okay, so is this PR ready for re-review, if all the commented issues are addressed? |
@pamelafox - Yes, all the commented issues have been addressed, and it's ready for re-review. Kindly requesting your feedback. |
I'd like to reduce the existing JS size before bringing this in, since this adds ~1MB of JS. I have a PR for that here: #1947 Otherwise, this PR is looking very good. I'll resolve the merge conflicts. |
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.
Great work! Thanks for your patience on the review.
Thank you so much! |
Purpose
This PR is to add multi-language support to the frontend of the app, which was previously only available in English. This new feature, along with i18next, will enable the app to automatically adapt to the user's browser language, thereby improving user's experience.
Does this introduce a breaking change?
When developers merge from main and run the server, azd up, or azd deploy, will this produce an error?
If you're not sure, try it out on an old environment.
Does this require changes to learn.microsoft.com docs?
This repository is referenced by this tutorial
which includes deployment, settings and usage instructions. If text or screenshot need to change in the tutorial,
check the box below and notify the tutorial author. A Microsoft employee can do this for you if you're an external contributor.
Type of change
Code quality checklist
python -m pytest --cov
to verify 100% coverage of added linespython -m mypy
to check for type errorsruff
andblack
manually on my code.See CONTRIBUTING.md for more details.