feat: Add automatic-translation extension - EXO-65429 #46
Conversation
hakermi
force-pushed
the
TASK-65429-1
branch
2 times, most recently
from
cdcbe23 to
5bfec4a
Compare
| extensionRegistry.registerExtension('automatic-translation', 'action', { | ||
| id: 'auto-translate', | ||
| rank: 1000, | ||
| isEnabled: () => true, | ||
| labelKey: 'UIActivity.label.translate', | ||
| click: (message, callback) => { | ||
| fetchTranslation(message).then(translated => { | ||
| callback(translated.translation); | ||
| }); | ||
| }, | ||
| }); |
There was a problem hiding this comment.
why we add this extension ?
for the translate button in notes and news ?
There was a problem hiding this comment.
Ok, the name of the extension is disturbing.
You named it 'automatic-translation', but should it be something like 'notes-translation' and 'news-translation' ?
For activities, the xtension is named "ActivityContent" or ('activity', 'comment-action').
There was a problem hiding this comment.
Ok, but i wanted to have one extension for both. that's why.
And i really don't know which is better: define one extension or two extensions(news-extension and notes-extension).
There was a problem hiding this comment.
By using only one extension point, you will not be able in the future to add something in the news extension point but not in the notes extension point.
For me, it is better to have 2 extensions point, with a different name
There was a problem hiding this comment.
Ok i will make two extensions
hakermi
force-pushed
the
TASK-65429-1
branch
2 times, most recently
from
1c5cc82 to
3399d68
Compare
| @@ -100,6 +100,17 @@ | |||
| }, | |||
| }); | |||
|
|
|||
| extensionRegistry.registerExtension('notes-automatic-translation', 'auto-translate', { | |||
There was a problem hiding this comment.
you named the extension point "notes-automatic-translation". Does that mean we only add autotranslation extension into ?
In fact, you named the extension point with a name which is related to the content of the extension, not of what is the extension point.
As the extension point is in notes, translation menu, I would name it "notes", with type 'translation-menu-extension'
There was a problem hiding this comment.
name of extension updated
This PR adds a new automatic extension point
|
Kudos, SonarCloud Quality Gate passed!
|
|
Your PR triggers too many exo-ci builds! Please finish your work and then, set your PR ready! Thank you |
This PR adds a new automatic extension point