Skip to content
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

UI doesn't removes attachment #315

Closed
r4sas opened this issue May 21, 2018 · 13 comments
Closed

UI doesn't removes attachment #315

r4sas opened this issue May 21, 2018 · 13 comments
Labels

Comments

@r4sas
Copy link
Member

r4sas commented May 21, 2018

According to changes in #298, reporting that Remove attachment doesn't works when image or file attached from clipboard.

Browser: Firefox ESR 52.5.4.

Reproducable on https://paste.i2pd.xyz/

@rugk
Copy link
Member

rugk commented May 21, 2018

We don't even support adding attachments from the clipboard, but anyway I can reproduce the "remove attachment" issue.

  1. Add a attachment.
  2. Click "New" button.
  3. Look at the attachment button again: It still lists the file name, there.

However, as far as I see, it is not uploaded anyway, when you click "Upload".

@rugk rugk added the bug label May 21, 2018
@rugk rugk changed the title UI doesn't removes attachment pasted from clipboard UI doesn't removes attachment May 21, 2018
@r4sas
Copy link
Member Author

r4sas commented May 21, 2018

I find it in other way:

  1. Click 'New' button.
  2. Paste attachment (file from clipboard or picture, like screenshot).
  3. Try remove attachment.
  4. Upload paste.

File will be uploaded because it not removed.
upd: attachments choosed in selection window removes correctly.

@rugk
Copy link
Member

rugk commented May 21, 2018

BTW

Paste attachment (file from clipboard or picture, like screenshot).

How do you do so?

@r4sas
Copy link
Member Author

r4sas commented May 21, 2018

@rugk if you about screenshot, simply using PrtScr key on keyboard. After that I pasted (ctrl+V) image in buffer to field where you must write text. Same way for files, by coping file in system.

@rugk
Copy link
Member

rugk commented May 21, 2018

https://paste.i2pd.xyz/ shows some NextCloud site BTW currently.

Ah okay yeah, the latest master seems to have that feature.

Anyway, i tried it by just using the file selector and it is essentially the same. The worst thing is, you don't see the image in the preview. You only see it when it is uploaded.

@r4sas
Copy link
Member Author

r4sas commented May 21, 2018

shows some NextCloud site BTW currently.

Aha, Thats because i reconfigure my DNS records now ;)

The worst thing is, you don't see the image in the preview.

Yes, same until I follow paste link,

@rugk
Copy link
Member

rugk commented May 21, 2018

https://paste.i2pd.xyz/ shows some NextCloud site BTW currently.

Ah okay yeah, the latest master seems to have that feature.

Anyway, i tried it by just using the file selector and it is essentially the same. The worst thing is, you don't see the image in the preview. You only see it when it is uploaded.

@rugk rugk closed this as completed in da45d34 May 21, 2018
@rugk
Copy link
Member

rugk commented May 21, 2018

So I fixed that, BTW the base confusion was that the attachment viewer is also used for storing to be uploaded attachments, which caused some confusion in handling them.
However, that's also very nice in order to easily re-upload attachments. But generally some refactoring here would be good, especially as some methods are not really clearly named "hasAttachmentData" BTW does not check, whether there is data to be uploaded, but whether there is data that needs to be decrypted (which is stored somewhere). It has actually nothing to do with the attachmentData variable, which is actually the "real" data of the attachment and also set when you create a new paste and add the file via the file selector.
@elrido, any idea? May a refactoring be useful/should I open an issue, at least?

@r4sas
Copy link
Member Author

r4sas commented May 21, 2018

Updated to latest version, got same issue.

Step-by-step what I do:

  1. Copy image to buffer.
  2. Paste (Ctrl+V) on new paste page.
    default
    (On picture default file choose button with "File not selected" message).
  3. Try remove attachment using Remove attachment button.

Attachment stay here (pic above) anyway.

@rugk
Copy link
Member

rugk commented May 21, 2018

Okay wait, maybe it is just the display. At least in my tests it was not uploaded anymore when I clicked "Send".

@rugk rugk reopened this May 21, 2018
@r4sas
Copy link
Member Author

r4sas commented May 21, 2018

Let me pull repo again... maybe last commit downloaded not correctly. Will update that message soon.

upd: yup, it not uploaded more, but that line with filename stings.
Also here we must handle situation, when file selected, and after that pasted or dropped in/on form.
That can make something like this:
default
In that case sending will upload what was added from pasted/dropped. Can we clear selection if pasted/dropped file, and, return second line to default value (info about ability) when file selected or attachment removed?

@rugk
Copy link
Member

rugk commented May 21, 2018

I'll have a look into the first display issue. As for what you wrote below: I am sorry, but I did not quite what the problem there is. It's best when you write "steps to reproduce", so I know what to do to get the problem.

@rugk rugk closed this as completed in 14a7fd7 May 21, 2018
@rugk
Copy link
Member

rugk commented May 21, 2018

So I found some things…

However there is one issue:

  • the files selected via file picker are shown at one place
  • the files via drag and drop below (as this is what you propbably wanted to show with your second screenshot)

As far as I see, this is hard to fix, because we cannot manualyl modify the file picker value from the outside for security reasons.

I've opened a new issue for that: #318

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants