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.

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

New Extension "TT": Translator #1009

Closed
wants to merge 26 commits into from
Closed

New Extension "TT": Translator #1009

wants to merge 26 commits into from

Conversation

deptno
Copy link

@deptno deptno commented Mar 1, 2022

Description

Add TT extension to translate texts with multiple providers.

  • From language, To language are configurable.
  • Multi language for TT extension will be supported.

Screencast

t.mov

Checklist

@raycastbot raycastbot added the new extension Label for PRs with new extensions label Mar 1, 2022
@raycastbot
Copy link
Collaborator

Congratulation on your new Raycast extension! 🚀

We will review it shortly. Once the PR is approved and merged, the extension will be available on the Store.

@deptno deptno changed the title New Extension "T": Translator New Extension "TT": Translator Mar 2, 2022
Copy link
Collaborator

@pernielsentikaer pernielsentikaer left a comment

Choose a reason for hiding this comment

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

Hi @deptno 👋

Thanks for your contribution 🔥, well done! I have some comments for you:

  • There is already an open PR with papago search Add Papago extension #888 started by @wooogle – it would make sense to use one of the extension as a base and both contribute.

Some feedback for your extension

  • At the moment, Raycast doesn't support localization and only supports US English.
  • The name of the command needs to be understandable, check Naming convention
  • Do I need to register for a key before “history” and does that make any sense?
  • We do already have Google Translate so it makes sense to make the Papago translater and focus on that?

Let me know your thoughts

@deptno
Copy link
Author

deptno commented Mar 5, 2022

Hi @pernielsentikaer

Thank you for your feedback. I reflected them in this extension.

  • At the moment, Raycast doesn't support localization and only supports US English.
    • I removed code related i18n.
  • The name of the command needs to be understandable, check Naming convention
    • I changed the name of the command to Translator.
  • Do I need to register for a key before “history” and does that make any sense?
    • No. You don't need to register for a key to use History. It isn't related.
  • We do already have Google Translate so it makes sense to make the Papago translater and focus on that?
    • Translation services are not accurate. so, It usually need to compare multiple services for better understanding.

Copy link
Collaborator

@pernielsentikaer pernielsentikaer left a comment

Choose a reason for hiding this comment

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

Hi @deptno 👋

Thanks for the update 💪

I have some suggestions and comments for you to address:

  • I'm not sure how the history works, tried a lot of searches but seems like nothing ends in the history?
  • Are you in Slack, would love to have an API key for 15 minutes to test it fully out.

Request a new review when you are ready, feel free to contact me here or at Slack if you have any questions.

extensions/tt/package.json Outdated Show resolved Hide resolved
{
"name": "index",
"title": "Transator",
"subtitle": "Translate texts",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
"subtitle": "Translate texts",
"subtitle": "Papago",

{
"$schema": "https://www.raycast.com/schemas/extension.json",
"name": "tt",
"title": "TT",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
"title": "TT",
"title": "Papago Translator",

@@ -0,0 +1,929 @@
{
"$schema": "https://www.raycast.com/schemas/extension.json",
"name": "tt",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
"name": "tt",
"name": "papago-translator",

extensions/tt/src/component/T.tsx Outdated Show resolved Hide resolved
deptno and others added 2 commits March 7, 2022 09:05
Co-authored-by: Per Nielsen Tikær <per@tikaer.dk>
Co-authored-by: Per Nielsen Tikær <per@tikaer.dk>
@deptno
Copy link
Author

deptno commented Mar 7, 2022

@pernielsentikaer

Thank you for your feedbacks.

  1. Purpose of the extension is to serve multiple translated texts from multiple providers(Google, Papago, etc) at once.

So, "Papago" can't be name of the extension.

  1. History doesn't work automatically, It is triggered by user action(cmd + enter or ActionPanel) and I'll changed confusing titles.

  2. check slack DM!

Copy link
Collaborator

@pernielsentikaer pernielsentikaer left a comment

Choose a reason for hiding this comment

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

Hi @deptno 👋

  1. Makes sense 😄
  2. Ah, seems like a section title caused the confusion

I have some suggestions for you:

  1. I would move the credentials to preferences instead, would fit how the rest of Raycast is working.
  2. After adding the key, then it seems like it's not happy fetching the result 😄

fG0lgifaD40tDszW

  1. Icon is missing in the ActionPanel here:

ZhqUeeST6Xz5XLLf

Request a new review when you are ready, feel free to contact me here or at Slack if you have any questions.

@deptno deptno marked this pull request as draft March 7, 2022 06:01
@deptno
Copy link
Author

deptno commented Mar 7, 2022

@pernielsentikaer

Thank you for feedbacks!

I agree that section title caused the confusion. so I removed History title from the section name.

I have question. Can you tell me your preferences values?

  • From
  • To

I converted PR status to draft. I will mention you again soon.

@pernielsentikaer
Copy link
Collaborator

I have question. Can you tell me your preferences values?

I selected Danish to English 😄

@pernielsentikaer
Copy link
Collaborator

Hi @deptno 👋

Do you have an update here?

@deptno
Copy link
Author

deptno commented Apr 24, 2022

Hi @pernielsentikaer

I'm not able to work on this now.
I will Pull request again later.

@deptno deptno closed this Apr 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants