Add simple attachment button

[newhinton]

Signed-off-by: Felix Nüsse [email protected]

This closes #74. It allows to upload arbitrary files to display as attachments. This should be superseeded and replaced by #790.

I did not add this button to the three dot menu because this would require the Note Vue component to hold references to the editor/view and react to it. This functionality however, will be part of #790.

On a sidenote, the uploadfeature may fail silently when the php upload size is set to lower than the file one tries to upload. The result is that the NoteService errors out because the 'tmp_name' field will be unset and fread cant read the file which has not been uploaded. Is this something we need to check against?

[korelstar]

Thanks for this PR!

On a sidenote, the uploadfeature may fail silently when the php upload size is set to lower than the file one tries to upload. The result is that the NoteService errors out because the 'tmp_name' field will be unset and fread cant read the file which has not been uploaded. Is this something we need to check against?

Yes, please! Could you please add additional checks for this to the PHP code and an error message to the JavaScript code?

Here are some other points:

• please implement the button using the ActionButton component inside of an Actions component. See e.g. https://github.com/nextcloud/notes/blob/v4.3.0/src/components/Note.vue#L41
• please add an appropriate icon (due to the text label, the button is too large) using vue-material-design-icons (e.g. file-image-plus?)
• please use position: fixed for positioning the button and place it directly (with an appropriate margin) left to the three-dot menu.
• could we also add another ActionButton (inside the same Actions) for adding existing images (your existing method onClickSelect)?
• can we add the original file name as alternate text for the markdown code? (e.g. ![original filename](123456.jpg))

Thanks!

[newhinton]

I implemented some of your changes!

We now have the buttons directly next to the threedot, also the select image button is there. However, i have reused available icons. The name used is now the original filename, not the generated uuid (as displayvalue, not the actual file)

I have also implemented a VERY basic check for tmp_file, though it really is not the best as it does not return a 500. If we want to expand on this, we should not do it in this pr because it requires quite a bit of changes.

I have also not used actionbuttons. I consider this PR a stopgap measure, and i dont want to spend too much time refining this pr since it gets removed anyway. (that obviously excludes the server-check and the errormessages which certainly do not get overridden by the toolbar. Also that is the reason why i didnt add new icons, they will be updated in #788)
I hope thats okay with you

Edit: I tried using actionbuttons as in the example, but that ended up in very strange behaviour that i didnt really want to debug, thats why i said i dont want to put too much effort in this

[korelstar]

Thanks for your changes and sorry for having too less time for a review. I've rebased the branch to current master and did some support with switching to the ActionButton and Actions components. For now, this part is fine.

I have also implemented a VERY basic check for tmp_file, though it really is not the best as it does not return a 500. If we want to expand on this, we should not do it in this pr because it requires quite a bit of changes.

I don't think that it will require much changes. You just need to throw an Exception in the PHP code, the method Controller.Helper.handleErrorResponse() does the remainder for you. But you will have to adjust the JavaScript code a little bit.

However, inserting an existing image has some issues. They exist already since #785:

• Insert existing image does not work, since the PROPFIND request results in a 404. I think it's because const currentNotePath = apppath + '/' + categories should use the current category instead of all categories
• even if this would work, the inserted Markdown code contains a wrong path to the chosen image - at least when working in a subdirectory

Could you please fix these two issues? The fix will be also needed for the toolbar PR, so it's not wasteful. Any changes will also apply to the toolbar PR (when you rebase it after merging this one).

[newhinton]

I implemented the 500 in the notesservice.

I like what you did with the action buttons, the only thing i changed was adding a 5px margin to seperate the two buttons.

[newhinton]

@korelstar Ah yes now i know why adding images fails: if i assume that store.getters.getCategories() returns the array that contains ALL available categories, not the category of the currently selected note. I fixed this by using the proper category. It should hopefully work now.

[korelstar]

There was still an issue when uploading an image (reference to this in function), but I fixed that.

In addition, there was an issue with the relative path (e.g. if note is in Test1/ and chosen image is in Test2/). I fundamentally re-worked this part by using the path module from NodeJS. Code is much shorter and less error prone, now.

All in all, I think it's done now. I will do some more tests and merge it then. Thanks!

[newhinton]

Oh yeah, that reference slipped, sorry.

Well, using a library is ingenious, somehow i always stick to plain js for most of the logic. Maybe it's the left-pad-trauma, but anyhow: yours looks way better!