-
Notifications
You must be signed in to change notification settings - Fork 7
feat: Implements web page translation #5
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: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -69,5 +69,4 @@ linux: | |
| - AppImage | ||
| icon: build/icons | ||
| category: Utility | ||
| desktop: | ||
| StartupWMClass: fluent-reader | ||
| desktop: {} | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -44,6 +44,7 @@ type ArticleProps = { | |
| type ArticleState = { | ||
| fontFamily: string | ||
| fontSize: number | ||
| translated: boolean | ||
| loadWebpage: boolean | ||
| loadFull: boolean | ||
| fullContent: string | ||
|
|
@@ -60,6 +61,7 @@ class Article extends React.Component<ArticleProps, ArticleState> { | |
| this.state = { | ||
| fontFamily: window.settings.getFont(), | ||
| fontSize: window.settings.getFontSize(), | ||
| translated: false, | ||
| loadWebpage: props.source.openTarget === SourceOpenTarget.Webpage, | ||
| loadFull: props.source.openTarget === SourceOpenTarget.FullContent, | ||
| fullContent: "", | ||
|
|
@@ -288,8 +290,7 @@ class Article extends React.Component<ArticleProps, ArticleState> { | |
| loadWebpage: | ||
| this.props.source.openTarget === SourceOpenTarget.Webpage, | ||
| loadFull: | ||
| this.props.source.openTarget === | ||
| SourceOpenTarget.FullContent, | ||
| this.props.source.openTarget === SourceOpenTarget.FullContent, | ||
| }) | ||
| if (this.props.source.openTarget === SourceOpenTarget.FullContent) | ||
| this.loadFull() | ||
|
|
@@ -304,6 +305,17 @@ class Article extends React.Component<ArticleProps, ArticleState> { | |
| if (refocus) refocus.focus() | ||
| } | ||
|
|
||
| toggleTranslation = () => { | ||
| if (this.state.translated) { | ||
| this.setState({ translated: false }) | ||
| } else if ( | ||
| this.props.item.link.startsWith("https://") || | ||
| this.props.item.link.startsWith("http://") | ||
| ) { | ||
| this.setState({ translated: true }) | ||
| } | ||
| } | ||
|
|
||
| toggleWebpage = () => { | ||
| if (this.state.loadWebpage) { | ||
| this.setState({ loadWebpage: false }) | ||
|
|
@@ -439,6 +451,18 @@ class Article extends React.Component<ArticleProps, ArticleState> { | |
| this.props.toggleStarred(this.props.item) | ||
| } | ||
| /> | ||
| {this.state.loadWebpage && ( | ||
| <CommandBarButton | ||
| title={ | ||
| this.state.translated | ||
| ? intl.get("article.untranslate") | ||
| : intl.get("article.translated") | ||
| } | ||
| className={this.state.translated ? "active" : ""} | ||
| iconProps={{ iconName: "Translate" }} | ||
| onClick={this.toggleTranslation} | ||
| /> | ||
| )} | ||
| <CommandBarButton | ||
| title={intl.get("article.loadFull")} | ||
| className={this.state.loadFull ? "active" : ""} | ||
|
|
@@ -477,7 +501,9 @@ class Article extends React.Component<ArticleProps, ArticleState> { | |
| } | ||
| src={ | ||
| this.state.loadWebpage | ||
| ? this.props.item.link | ||
| ? this.state.translated | ||
| ? `https://translate.google.com/translate?sl=auto&tl=${this.props.locale.split("-")[0]}&u=${encodeURIComponent(this.props.item.link)}` | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Probably can be sectioned off into its other util function.
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
To have a code that is easier to read, or is it something more technical?
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. More for expanding to different translation backends. Thinking about what this feature needs and how to make it ergonomic... |
||
| : this.props.item.link | ||
| : this.articleView() | ||
| } | ||
| allowpopups={"true" as unknown as boolean} | ||
|
|
||
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.
This check is odd to put here. I assume this is needed for maybe ftp or gopher links? Do you know why this is here? what other protocols has fluent reader supported in the past?
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.
I don't have much idea, this was not an open PR with a detailed description. Is it unnecessary? if so, is it safe to remove it or do you have to refactor it?
About protocols, tbh, idk, sorry
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.
No worries! I'll look around in the codebase to try to understand what this is doing first.