-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Add export as txt/md #245
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 export as txt/md #245
Conversation
|
That’s awesome! We'll check it. Cheers👏 |
| import Raphael from 'raphael' | ||
| import flowchart from 'flowchart' | ||
| import SequenceDiagram from 'js-sequence-diagrams' | ||
| import ee from 'browser/main/lib/eventEmitter' |
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.
Please do not omit the name of variable. For example, use eventEmitter.
| const { remote } = require('electron') | ||
| const { app } = remote | ||
| const path = require('path') | ||
| const dialog = remote.require('electron').dialog |
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.
electron seems to be required twice.
If you import it on the first line, you don't need require twice.
import { remote, dialog } from 'electron'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.
OMG, I don't have to require twice 😨. However the code cannot operate correctly because I want to use dialog in the renderer process. Thus I'll change it to const dialog = remote.dialog
The Dialog is opened from Electron's main thread. If you want to use the dialog object from a renderer process, remember to access it using the remote:
const {dialog} = require('electron').remote console.log(dialog)
refs: https://github.com/electron/electron/blob/master/docs/api/dialog.md
| dialog.showSaveDialog(remote.getCurrentWindow(), options, | ||
| (filename) => { | ||
| if (filename) { | ||
| fs.writeFile(filename, this.props.value, () => {}) |
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.
Please handle error by using callback function:bow:
| label: 'Export as', | ||
| submenu: [ | ||
| { | ||
| label: 'Plain Text (.txt)', |
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.
You can write like below. It is more modern way.
click () {
}|
Thank you for your contribution! Please fix some points 🐱 |
|
@sota1235 Thanks so much for your review 🙏 🙏 🙏, I'll fix them 🙏 🙏 🙏 |
|
I fixed them 🙏 |
|
Thank you for fixing! |
|
Thanks for you review so much 🙏 I'll handle it! |
browser/main/NoteList/index.js
Outdated
|
|
||
| alertIfSnippet() { | ||
| let { location } = this.props | ||
| let targetIndex = _.findIndex(this.notes, (note) => { |
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.
Please use const instead of let!
browser/main/NoteList/index.js
Outdated
| alertIfSnippet() { | ||
| let { location } = this.props | ||
| let targetIndex = _.findIndex(this.notes, (note) => { | ||
| return note.storage + '-' + note.key === location.query.key |
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.
You can use template literal!
return `${note.storage}-${note.key}` === location.query.key|
Tha's all from me. Please fix, then I'll merge 🙇 |
|
@sota1235 Thanks for your review! I fixed the code you pointed 🙏 |
|
Thank you for fixing. Looks good to me! |

Hi, I implemented a function which exports as txt or md. I heard it was said that someone wants this function.