-
Notifications
You must be signed in to change notification settings - Fork 3.4k
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
Add lingo-rep-raycast extension #16512
base: main
Are you sure you want to change the base?
Conversation
- fix warnings - fix lint errors - prepare prod build
Congratulations on your new Raycast extension! 🚀 Due to our current reduced availability, the initial review may take up to 10-15 business days Once the PR is approved and merged, the extension will be available on our Store. |
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.
Hi 👋
Thanks for your contribution 💪
I have now tested your extension, and I have some feedback ready for you:
- Could you make a screencast of the flow so I can see how it works 😊
I'm looking forward to testing this extension again 🔥
Request a new review when you are ready. Feel free to contact me here or at Slack if you have any questions.
Co-authored-by: Per Nielsen Tikær <per@raycast.com>
Co-authored-by: Per Nielsen Tikær <per@raycast.com>
@pernielsentikaer thank you for getting back to me! I've added screencast and description - please check them out |
"copy:shared-packages": "./copy-shared-packages.sh", | ||
"watch:translate": "chokidar '../../packages/translate/dist/**/*' -c 'npm run copy:shared-packages'", | ||
"watch:lang-options": "chokidar '../../packages/lang-options/dist/**/*' -c 'npm run copy:shared-packages'", | ||
"watch:shared-packages": "npm-run-all --parallel watch:translate watch:lang-options", | ||
"npm:install": "npm install --prefix /Users/alex/Work/Projects/alex/lingo-rep-monorepo/apps/lingo-rep-raycast-extension", | ||
"build": "npm run npm:install && ray build -e dist", | ||
"dev:raycast": "ray develop", | ||
"dev": "npm run npm:install && npm run copy:shared-packages && npm-run-all --parallel watch:shared-packages dev:raycast", | ||
"dev-raycast": "", | ||
"fix-lint": "ray lint --fix", | ||
"lint": "ray lint", | ||
"publish": "npx @raycast/api@latest publish" |
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.
Could we just have the default ones?
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.
@pernielsentikaer changed, please take a look
hey @pernielsentikaer , I also shortened my repo URL from Will it be ok to publish it like that? Won't it cause issues when I submit new versions? |
oAuth is not meant to be used like that with multiple services, did you see this page? When I login I'm try to login it just ends in this view Is there a reason why you need to login, it should more be a login to LingoRep.com? |
hey @pernielsentikaer, thanks for getting back to me.
I've made some changes - now after login user is redirected to the main view of the Raycast, please take a look. As for the necessity of login - in our web app we use Google and Github to authenticate users. Thus I've implemented them here as well. Without being authenticated users won't be able to save translations to the database. The idea is as following:
Cheers |
It feels a bit weird from a user perspective getting the actions when you're first entering the extension - I'm not sure if we can do anything to make this a better experience |
hey @pernielsentikaer , could you elaborate more on Currently, initial flow for user is:
Logging in - is optional step for user, if they want to save and repeat words later. That's what is suggested in the actions section |
It might be because I have no clue how it really works Tried to login with Google and hit save and got this error:
Do you know what causes this? |
hey @pernielsentikaer , thanks for details and for QAing it thoroughly! It was a bug with authorization in my code. I just fixed it and pushed a new version. Could you take a look? |
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.
PR Summary
This PR adds a new LingoRep extension for Raycast that enables text translation with spaced repetition learning capabilities, featuring OAuth authentication and multi-language support.
- The CHANGELOG.md should use
{PR_MERGE_DATE}
instead of a hardcoded date2024-05-05
- The extension uses an unofficial Google Translate API endpoint (
client=gtx
) in/src/shared-packages/translate/index.js
which may have stability/usage limitations - The
config.ts
contains a commented "test" line that should be removed - The
request.js
implementation lacks proper error handling for HTTP requests - The
README.md
contains typos ("usign" should be "using") and could benefit from improved formatting
💡 (1/5) You can manually trigger the bot by mentioning @greptileai in a comment!
22 file(s) reviewed, 29 comment(s)
Edit PR Review Bot Settings | Greptile
@@ -0,0 +1,3 @@ | |||
# LingoRep Changelog | |||
|
|||
## [Initial Version] - 2024-05-05 |
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.
syntax: Use {PR_MERGE_DATE} placeholder instead of hardcoded date. This will be automatically replaced with the current date when the PR is merged.
## [Initial Version] - 2024-05-05 | |
## [Initial Version] - {PR_MERGE_DATE} |
"axios": "1.7.2", | ||
"hono": "^4.4.2", | ||
"lodash": "^4.17.21", | ||
"node-fetch": "^3.3.2", |
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.
style: Both axios and node-fetch are included - consider standardizing on one HTTP client
@@ -0,0 +1,20 @@ | |||
let nodeEnv = process.env.NODE_ENV as unknown as "development" | "production" | undefined; |
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.
style: Type assertion chain is unsafe. Consider using proper type guards or environment validation
weURL: "chrome-extension://gfmbkbpbncjopblehgldppphpkcmehnk/settings.html", | ||
}, | ||
}; | ||
// test |
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.
style: Remove empty test comment
// test |
}; | ||
// test | ||
|
||
export const config = allConfigs[nodeEnv]; |
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.
logic: Missing runtime check - config could be undefined if nodeEnv is invalid
export const config = allConfigs[nodeEnv]; | |
if (!config) throw new Error(`Invalid environment: ${nodeEnv}`); | |
export const config = allConfigs[nodeEnv]; |
"Content-Type": "application/json", | ||
}, | ||
data: data && method === "POST" ? JSON.stringify(data) : undefined, | ||
url: url ? url : "", |
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.
logic: Empty URL fallback could cause invalid requests. Consider throwing an error if URL is empty
sq: string; | ||
ar: string; | ||
hy: string; | ||
az: string; | ||
eu: string; | ||
be: string; | ||
bn: string; | ||
bs: string; | ||
bg: string; | ||
ca: string; | ||
ceb: string; | ||
ny: string; | ||
"zh-cn": string; | ||
"zh-tw": string; | ||
co: string; | ||
hr: string; | ||
cs: string; | ||
da: string; | ||
nl: string; | ||
eo: string; | ||
et: string; | ||
tl: string; | ||
fi: string; | ||
fr: string; | ||
fy: string; | ||
gl: string; | ||
ka: string; | ||
de: string; | ||
el: string; | ||
gu: string; | ||
ht: string; | ||
ha: string; | ||
haw: string; | ||
iw: string; | ||
hi: string; | ||
hmn: string; | ||
hu: string; | ||
is: string; | ||
ig: string; | ||
id: string; | ||
ga: string; | ||
it: string; | ||
ja: string; | ||
jw: string; | ||
kn: string; | ||
kk: string; | ||
km: string; | ||
ko: string; | ||
ku: string; | ||
ky: string; | ||
lo: string; | ||
la: string; | ||
lv: string; | ||
lt: string; | ||
lb: string; | ||
mk: string; | ||
mg: string; | ||
ms: string; | ||
ml: string; | ||
mt: string; | ||
mi: string; | ||
mr: string; | ||
mn: string; | ||
my: string; | ||
ne: string; | ||
no: string; | ||
ps: string; | ||
fa: string; | ||
pl: string; | ||
pt: string; | ||
ma: string; | ||
ro: string; | ||
ru: string; | ||
sm: string; | ||
gd: string; | ||
sr: string; | ||
st: string; | ||
sn: string; | ||
sd: string; | ||
si: string; | ||
sk: string; | ||
sl: string; | ||
so: string; | ||
su: string; | ||
sw: string; | ||
sv: string; | ||
tg: string; | ||
ta: string; | ||
te: string; | ||
th: string; | ||
tr: string; | ||
ur: string; | ||
uz: string; | ||
vi: string; | ||
cy: string; | ||
xh: string; | ||
yi: string; | ||
yo: string; | ||
zu: string; | ||
}; |
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.
style: sourceLangOptions and sourceLangOptionsRaycast contain identical language mappings. Consider consolidating into a single shared object to reduce duplication.
sq: string; | ||
ar: string; | ||
hy: string; | ||
az: string; | ||
eu: string; | ||
be: string; | ||
bn: string; | ||
bs: string; | ||
bg: string; | ||
ca: string; | ||
ceb: string; | ||
ny: string; | ||
"zh-cn": string; | ||
"zh-tw": string; | ||
co: string; | ||
hr: string; | ||
cs: string; | ||
da: string; | ||
nl: string; | ||
eo: string; | ||
et: string; | ||
tl: string; | ||
fi: string; | ||
fr: string; | ||
fy: string; | ||
gl: string; | ||
ka: string; | ||
de: string; | ||
el: string; | ||
gu: string; | ||
ht: string; | ||
ha: string; | ||
haw: string; | ||
iw: string; | ||
hi: string; | ||
hmn: string; | ||
hu: string; | ||
is: string; | ||
ig: string; | ||
id: string; | ||
ga: string; | ||
it: string; | ||
ja: string; | ||
jw: string; | ||
kn: string; | ||
kk: string; | ||
km: string; | ||
ko: string; | ||
ku: string; | ||
ky: string; | ||
lo: string; | ||
la: string; | ||
lv: string; | ||
lt: string; | ||
lb: string; | ||
mk: string; | ||
mg: string; | ||
ms: string; | ||
ml: string; | ||
mt: string; | ||
mi: string; | ||
mr: string; | ||
mn: string; | ||
my: string; | ||
ne: string; | ||
no: string; | ||
ps: string; | ||
fa: string; | ||
pl: string; | ||
pt: string; | ||
ma: string; | ||
ro: string; | ||
ru: string; | ||
sm: string; | ||
gd: string; | ||
sr: string; | ||
st: string; | ||
sn: string; | ||
sd: string; | ||
si: string; | ||
sk: string; | ||
sl: string; | ||
so: string; | ||
su: string; | ||
sw: string; | ||
sv: string; | ||
tg: string; | ||
ta: string; | ||
te: string; | ||
th: string; | ||
tr: string; | ||
ur: string; | ||
uz: string; | ||
vi: string; | ||
cy: string; | ||
xh: string; | ||
yi: string; | ||
yo: string; | ||
zu: string; | ||
}; |
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.
style: targetLangOptions and targetLangOptionsRaycast contain identical language mappings. Consider consolidating into a single shared object to reduce duplication.
fa: string; | ||
pl: string; | ||
pt: string; | ||
ma: string; |
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.
logic: 'ma' appears to be an invalid ISO language code. Should likely be 'mr' for Marathi.
ma: string; | |
mr: string; |
ht: string; | ||
ha: string; | ||
haw: string; | ||
iw: string; |
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.
style: 'iw' is a deprecated language code. Should use 'he' for Hebrew instead.
iw: string; | |
he: string; |
07d5b7e
to
5cff160
Compare
Description
The LingoRep Raycast extension makes it easy to translate text between languages. Anyone can use it for quick translations. Once you sign in, you can save your translations and use the spaced repetition algorithm in the web app to help memorize them effectively. More about it on the web site - https://lingorep.com/
Screencast
Screencast video
Checklist
npm run build
and tested this distribution build in Raycastassets
folder are used by the extension itselfREADME
are placed outside of themetadata
folder