-
Notifications
You must be signed in to change notification settings - Fork 149
Add: Copy to clipboard button for translated text #154
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
base: main
Are you sure you want to change the base?
Add: Copy to clipboard button for translated text #154
Conversation
WalkthroughThis update refines the Chrome extension's structure by specifying concrete metadata and permissions in the manifest, introducing a new package.json, and simplifying the popup's welcome page. Inline styles and scripts are moved to external files, and the theme toggle and API key logic in the welcome page are streamlined for clarity. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant WelcomePage (welcome.html)
participant WelcomeScript (welcome.js)
participant LocalStorage
User->>WelcomePage: Loads welcome.html
WelcomePage->>WelcomeScript: Loads as module
WelcomeScript->>LocalStorage: Reads stored theme and API key
User->>WelcomeScript: Clicks "Save" after entering API key
alt API key input is not empty
WelcomeScript->>LocalStorage: Saves API key
WelcomeScript->>User: Shows "API key saved" alert
else API key input is empty
WelcomeScript->>User: Shows "Please enter a valid API key" alert
end
User->>WelcomeScript: Clicks theme toggle
WelcomeScript->>WelcomePage: Updates theme class
WelcomeScript->>LocalStorage: Saves new theme
Possibly related PRs
Poem
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 0
🧹 Nitpick comments (3)
package.json (2)
5-8: Reconsider the main entry point and start script for a Chrome extension.The
mainfield points tobackground.jsand thestartscript attempts to run it with Node.js. However,background.jsis designed as a Chrome extension service worker and won't function correctly in a Node.js environment.For Chrome extensions, consider removing the
mainfield or setting it to a different file that's meant for Node.js usage.- "main": "background.js", "scripts": { - "start": "node background.js", + "start": "echo 'This is a Chrome extension. Load it in Chrome developer mode.'", "test": "echo \"Error: no test specified\" && exit 1" },
10-11: Consider populating the empty metadata fields.The
keywordsandauthorfields are empty. Adding relevant keywords and author information would improve the package discoverability and documentation.- "keywords": [], - "author": "", + "keywords": ["chrome-extension", "translator", "hinglish", "ai"], + "author": "R-sarvankar",popup/welcome.js (1)
4-9: Improve theme initialization robustness.The theme initialization could be more robust by handling the case where no theme is saved or an invalid theme is stored.
// Load saved theme from localStorage -const savedTheme = localStorage.getItem('theme'); -if (savedTheme) { - body.className = savedTheme; +const savedTheme = localStorage.getItem('theme') || 'light-mode'; +if (savedTheme === 'dark-mode' || savedTheme === 'light-mode') { + body.className = savedTheme; themeToggle.textContent = savedTheme === 'dark-mode' ? '☀️' : '🌙'; +} else { + // Fallback to light mode if invalid theme is stored + body.className = 'light-mode'; + themeToggle.textContent = '🌙'; + localStorage.setItem('theme', 'light-mode'); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (5)
content.js(1 hunks)manifest.json(1 hunks)package.json(1 hunks)popup/welcome.html(2 hunks)popup/welcome.js(1 hunks)
🔇 Additional comments (10)
content.js (1)
167-167: Good practice: Adding newline at end of file.This formatting improvement ensures the file ends with a newline, which is a good practice for text files.
popup/welcome.html (3)
4-6: Good refactoring: External stylesheet and proper self-closing tags.The changes properly separate concerns by moving inline CSS to an external stylesheet and using self-closing tags consistently.
22-23: Good practice: Loading JavaScript as a module.Loading the external script with
type="module"is a good practice that enables modern JavaScript features.
1-25: Inconsistency: No clipboard functionality found.The PR objectives state that this pull request adds a "Copy to Clipboard" button for translated text, but no such functionality is visible in this HTML file. The changes appear to be structural refactoring rather than adding clipboard features.
Please verify if the clipboard functionality was implemented in a different file or if the PR objectives description is incorrect.
Likely an incorrect or invalid review comment.
manifest.json (3)
2-8: Well-structured manifest with appropriate metadata.The manifest properly defines the extension with correct metadata, version, and permissions. The permissions are appropriately scoped for the translator functionality.
15-18: Good practice: Service worker as module.Configuring the background script as a service worker module aligns with Manifest V3 best practices.
6-6: Consider reviewing permission scope.The extension requests broad permissions including "tabs" and "activeTab". Ensure these are necessary for the current functionality. If the extension only needs to access specific tabs or content, consider if the permissions can be more restrictive.
Please verify if all these permissions are actually used by the extension:
tabs- for accessing tab informationactiveTab- for accessing the currently active tabscripting- for injecting scripts into pagespopup/welcome.js (3)
11-21: Good refactoring: Cleaner theme toggle implementation.The use of
classList.replace()is more elegant than adding/removing classes, and the logic is simplified and easier to understand.
27-36: Simplified API key handling.The refactored API key handling is simpler and more straightforward, removing unnecessary complexity from the original implementation.
1-36: Critical inconsistency: Missing clipboard functionality.The PR objectives clearly state that this pull request adds a "Copy to Clipboard" button for translated text, but no such functionality is implemented in this file or any of the reviewed files. The changes appear to be structural refactoring rather than adding the advertised clipboard feature.
Please verify if:
- The clipboard functionality was implemented in a different file not included in this review
- The PR objectives description is incorrect
- The clipboard functionality is planned for a future commit in this PR
#!/bin/bash # Search for clipboard-related functionality in the codebase rg -i "clipboard|copy.*button|copy.*text" --type js --type html --type cssLikely an incorrect or invalid review comment.
✅ Added a "Copy to Clipboard" button under the translated text in the popup.
➕ Users can now easily copy translated Hinglish output to paste/share elsewhere.
📁 Changes made in: welcome.html, welcome.js
Summary by CodeRabbit