Internationalization #193
Open
Internationalization #193
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Vedansh Saini <77830698+vedansh-5@users.noreply.github.com>
Signed-off-by: Vedansh Saini <77830698+vedansh-5@users.noreply.github.com>
Reviewer's GuideThis PR implements internationalization by annotating UI elements in popup.html with data-i18n attributes, introducing locale JSON files under src/_locales, and wiring up translation logic in popup.js using the chrome.i18n API, along with updating manifest.json to specify the default locale. File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @vedansh-5 - I've reviewed your changes and they look great!
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `src/scripts/popup.js:240` </location>
<code_context>
// Allow empty org to fetch all GitHub activities
chrome.storage.local.set({ orgName: org }, () => {
- generateBtn.innerHTML = '<i class="fa fa-spinner fa-spin"></i> Generating...';
+ generateBtn.innerHTML = `<i class="fa fa-spinner fa-spin"></i> ${chrome.i18n.getMessage('generatingButton')}`;
generateBtn.disabled = true;
window.generateScrumReport();
</code_context>
<issue_to_address>
No fallback for missing 'generatingButton' i18n key.
If the i18n key is missing, only the spinner appears. Please add a fallback string to improve user experience.
</issue_to_address>
<suggested_fix>
<<<<<<< SEARCH
generateBtn.innerHTML = `<i class="fa fa-spinner fa-spin"></i> ${chrome.i18n.getMessage('generatingButton')}`;
=======
const generatingText = chrome.i18n.getMessage('generatingButton') || 'Generating...';
generateBtn.innerHTML = `<i class="fa fa-spinner fa-spin"></i> ${generatingText}`;
>>>>>>> REPLACE
</suggested_fix>
### Comment 2
<location> `src/scripts/popup.js:458` </location>
<code_context>
const scrumReport = document.getElementById('scrumReport');
if (scrumReport) {
- scrumReport.innerHTML = '<p style="text-align: center; color: #666; padding: 20px;">Organization changed. Click Generate button to fetch the GitHub activities.</p>';
+ scrumReport.innerHTML = `<p style="text-align: center; color: #666; padding: 20px;">${chrome.i18n.getMessage('orgChangedMessage')}</p>`;
}
// Clear the githubCache for previous org
</code_context>
<issue_to_address>
No fallback for missing 'orgChangedMessage' i18n key.
Without a fallback, the report may display nothing if the key is missing. Please add a default message to ensure the UI remains informative.
</issue_to_address>
<suggested_fix>
<<<<<<< SEARCH
if (scrumReport) {
scrumReport.innerHTML = `<p style="text-align: center; color: #666; padding: 20px;">${chrome.i18n.getMessage('orgChangedMessage')}</p>`;
}
=======
if (scrumReport) {
const orgChangedMsg = chrome.i18n.getMessage('orgChangedMessage') || 'Organization changed. Click Generate button to fetch the GitHub activities.';
scrumReport.innerHTML = `<p style="text-align: center; color: #666; padding: 20px;">${orgChangedMsg}</p>`;
}
>>>>>>> REPLACE
</suggested_fix>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Signed-off-by: Vedansh Saini <77830698+vedansh-5@users.noreply.github.com>
|
@vedansh-5 it doesn't work, please see screenshot |
That's really weird, because |
|
@Preeti9764 Could you also review this and see if the same error persists for you. Thanks |
Signed-off-by: Vedansh Saini <77830698+vedansh-5@users.noreply.github.com>
|
@Preeti9764 Please take a look whenever you can, thanks, |
|
@Preeti9764 Please review this PR and share your views on this. Thanks! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
📌 Fixes
Fixes #159
📝 Summary of Changes
All the labels of extension changes according to the default language set in users' browser.
📸 Screenshots / Demo (if UI-related)
✅ Checklist
👀 Reviewer Notes
Add any special notes for the reviewer here
Summary by Sourcery
Externalize all user-facing text into localized message files and apply internationalization across the extension’s UI based on the user’s browser language.
New Features:
Enhancements: