Skip to content

Conversation

@asmsuechan
Copy link
Contributor

This PR enable that to copy an image when it is dropped into CodeEditor. It means the image is kept into boostnote storage: ~/Boostnote/images/imageName.png

@asmsuechan
Copy link
Contributor Author

related: #286

@asmsuechan asmsuechan changed the title [WIP] Add dataApi.copyImage for copying image to boostnote storage on an image dropped into CodeEditor Add dataApi.copyImage for copying image to boostnote storage on an image dropped into CodeEditor Feb 21, 2017
@asmsuechan
Copy link
Contributor Author

I'm waiting to be merged #298.

this.insertImage(imageMd)

copyImage(imagePath, this.props.storageKey).then((imagePathInTheStorage) => {
let imageMd = `![${encodeURI(filename)}](${imagePathInTheStorage})`
Copy link
Contributor Author

@asmsuechan asmsuechan Feb 21, 2017

Choose a reason for hiding this comment

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

I changed this let to const in #298.

insertImage (imageMd) {
const textarea = this.editor.getInputField()
textarea.value = textarea.value.substr(0, textarea.selectionStart) + imageMd + textarea.value.substr(textarea.selectionEnd)
insertImageMd (imageMd) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Good Rename!

const textarea = this.editor.getInputField()
textarea.value = textarea.value.substr(0, textarea.selectionStart) + imageMd + textarea.value.substr(textarea.selectionEnd)
insertImageMd (imageMd) {
const cm = this.editor
Copy link
Contributor

Choose a reason for hiding this comment

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

What the mean of cm? If it is not popular wording, I want you to explain more detail.

Copy link
Contributor Author

@asmsuechan asmsuechan Mar 19, 2017

Choose a reason for hiding this comment

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

it's CodeMirror and used this name overall of Boostnote.

if (!_.isArray(cachedStorageList)) throw new Error('Target storage doesn\'t exist.')

const storage = _.find(cachedStorageList, {key: storageKey})
if (storage == null) throw new Error('Target storage doesn\'t exist.')
Copy link
Contributor

Choose a reason for hiding this comment

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

Can't you use ===?
And maybe, _.find returns undefined.
https://lodash.com/docs/4.17.4#find
So you need to write === undefined.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Alright

if (storage == null) throw new Error('Target storage doesn\'t exist.')
return storage
} catch (e) {
return Promise.reject(e)
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe you can write like below.

try {
  // Try to get target storage
  const targetStorage = 'brabra';
} catch (e) {
  // You do not need to try creating 'images' directory
  return Promise.reject(e);
}

return new Promise((resolve, reject) => {
  // brabra
});

Copy link
Contributor

@sota1235 sota1235 left a comment

Choose a reason for hiding this comment

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

Please fix some points.

@asmsuechan
Copy link
Contributor Author

@sota1235 I fixed them.

@jasondavis
Copy link

This is going to be a great feature that many people want!

Copy link
Contributor

@sota1235 sota1235 left a comment

Choose a reason for hiding this comment

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

LGTM

@sota1235 sota1235 merged commit 0862c6e into BoostIO:master Jun 4, 2017
@asmsuechan asmsuechan deleted the copy-image-on-dropped-into-CodeEditor branch July 22, 2017 06:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants