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 attachment to note #1723

Closed
ehhc opened this Issue Mar 22, 2018 · 9 comments

Comments

Projects
None yet
3 participants
@ehhc
Copy link
Contributor

ehhc commented Mar 22, 2018

Hi guys,
i’ve seen that #250 introduced a way to drop images to the markdown editor to get it inserted as an attachment. It is then also copied to the /images folder in the configured storage folder.
It would be quite nice to have this feature for additional attachment types as well. E.g. I would like to include PDF files as attachment in notes. For me it would be enough if they would be included as a link that can be opened by the OS (e.g. a file:// link).
Any Ideas how to do that?
Background of the question is that my storage folder is within a folder synchronized by a cloud client and I want to include attachments in it as well..
Thanks for your help :)

@ehhc

This comment has been minimized.

Copy link
Contributor

ehhc commented Mar 22, 2018

Attached to this issue is a very quick-and-dirty solution for the given problem. Alas I still do have a lot of questions, I can’t figure out (resp. a lot of issues I stumbled over):

  1. The html export of notes containing images is broken -> no images present
  2. Markdown export with embed files/images is broken -> links are broke
  3. Why are the dropped files named with Math.random().toString(36).slice(-16) (CopyImages.js) instead of something more resilient like Hash(Timestamp + filename) ? Math.random() might return the same result again. Especially when not properly initialized!
  4. Why are there (at various places) different definitions of markdown-templates to include images? I’ve found at least 3 without being able to figure out why they aren’t at one central place and in what cases they are used
  5. Why are there multiple definitions of the image folder path? e.g. moveNote.js:77, exportNote.js:9, copyImage.js:22 and none global definition? A global reliable definition of this constant variable is missing
  6. No documentation (e.g. JavaScript Doc) is provided. You have to guess what a method does and why it is necessary

Further issues coming up with my dirty fix hack:

  • methods aren’t doing what they promise to do: e.g. copyImage is able to and use for copying abbreviated files.
  • “Images” folder (see 5) ) contains further non images attachments as well

I hope you can understand my surprise/irritation about these various issues..

@ehhc

This comment has been minimized.

Copy link
Contributor

ehhc commented Mar 23, 2018

@Rokt33r ans ideas about this?

@Rokt33r

This comment has been minimized.

Copy link
Member

Rokt33r commented Mar 23, 2018

Yea, I think most things you've pointed make a sense. There are lots of things to be fixed because I'd been absent for a year. 😭 😭

Could you help us? It should be grateful if you can contribute our project.

@ehhc

This comment has been minimized.

Copy link
Contributor

ehhc commented Mar 25, 2018

Hi,
Thanks for your reply. Even though, i would really love to contribute, I'm not sure whether I've enough time to do so at the moment.
I will keep that in mind and try to come back to it in the next time.. Maybe i might find some spare time..
Until then: Can you have a look at my proposed "hack"? Might it possible to merge it? Even though ugly, at least it fixes some of the issues currently present..

@Rokt33r

This comment has been minimized.

Copy link
Member

Rokt33r commented Mar 25, 2018

It's okay. If no one has time to do, I'll take care about it.

Can you have a look at my proposed "hack"? Might it possible to merge it? Even though ugly, at least it fixes some of the issues currently present..

I'm going to review it in this week.

@nlopin

This comment has been minimized.

Copy link
Contributor

nlopin commented Mar 28, 2018

The html export of notes containing images is broken -> no images present
Markdown export with embed files/images is broken -> links are broke

Do you use the last version?

All exports were fixed some time ago. If you do use the last version, please, provide a video and console output after the export.

@ehhc

This comment has been minimized.

Copy link
Contributor

ehhc commented Mar 29, 2018

Hi there, Thanks for your reply @nlopin
I'm using version 0.11.3.
If i export the following note (markdown):

# Bla
test
![cat_scaled.jpg](\:storage\0.950lu04aev7.jpg)

[AR.txt](\:storage\0.9ra1y6n1euk.txt)

I get the following html:

<html>
  <head>
    <meta charset="UTF-8">
    <meta name = "viewport" content = "width = device-width, initial-scale = 1, maximum-scale = 1">
    <!-- irrelevant style information -->
    <link rel="stylesheet" href="css/dracula.css"><link rel="stylesheet" href="css/katex.min.css"><link rel="stylesheet" href="css/codemirror.css">
  </head>
  <body>
    <h1 data-line="0" id="Bla">Bla</h1>
    <p data-line="1">test<br />
    <img src=":storage%5C0.950lu04aev7.jpg" alt="cat_scaled.jpg" /></p>
    <p data-line="4"><a href=":storage%5C0.9ra1y6n1euk.txt">AR.txt</a></p>
  </body>
</html>

As you can see the links refer to the location ":storage%5C" which is not available on my desktop!
There are no errors in the console and i hope you can understand this issue without a video (i don't have any idea how to do a video expressing more than the attached code here..

Thanks for your help

@Rokt33r

This comment has been minimized.

Copy link
Member

Rokt33r commented Apr 3, 2018

@nlopin I think ehhc is using windows because the pathname is including back tick(`).

@Rokt33r

This comment has been minimized.

Copy link
Member

Rokt33r commented Apr 3, 2018

It seems the bug has been reported already. #1628

@Rokt33r Rokt33r closed this in cf77608 Apr 10, 2018

Rokt33r added a commit that referenced this issue Apr 10, 2018

pyriand3r pushed a commit to pyriand3r/Boostnote that referenced this issue Jun 19, 2018

pyriand3r pushed a commit to pyriand3r/Boostnote that referenced this issue Jun 19, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment