focalboard icon indicating copy to clipboard operation
focalboard copied to clipboard

Published 20 hours ago • verified publisher icon mattermost

File attachment in the card

Open Rajat-Dabade opened this issue 2 years ago • 8 comments

Summary

This PR adds the functionality to attach files of any type to the card.

Rajat-Dabade avatar Oct 20 '22 09:10 Rajat-Dabade

/update-branch

harshilsharma63 avatar Oct 25 '22 04:10 harshilsharma63

Error trying to update the PR. Please do it manually.

mattermod avatar Oct 25 '22 04:10 mattermod

  1. Downloading attached files always results in them being downloaded with .jpeg file extension.
  2. @asaadmahmood because of narrow card width, not even 2 attachment cards are visible at a time. It looks a bit odd (to me :P). How about making the card just wide enough to allow displaying two attachment cards and wrapping to allow, say, 3-4 rows?

Original- Screenshot 2022-10-25 at 11 35 33 AM

Wider card with wrapping- Screenshot 2022-10-25 at 11 37 51 AM

  1. Can't select multiple files at once.

  2. The detail and download icons aren't right aligned- Screenshot 2022-10-25 at 12 01 29 PM

  3. Options menu is cut/hidden when opening for the leftmost card- Screenshot 2022-10-25 at 12 07 47 PM

harshilsharma63 avatar Oct 25 '22 09:10 harshilsharma63

@harshilsharma63 I don't mind making the card slightly wider, but I think the scroll is fine, because then the attachments don't push other content down if a user does not care about them.

asaadmahmood avatar Oct 25 '22 12:10 asaadmahmood

Adding as content type feels weird. I don't think we should add in content section.

https://user-images.githubusercontent.com/12704875/197894936-f15563f3-dfa7-469f-9746-3ba1d0c38c3d.mov

sbishel avatar Oct 25 '22 22:10 sbishel

@harshilsharma63 I don't mind making the card slightly wider, but I think the scroll is fine, because then the attachments don't push other content down if a user does not care about them.

@asaadmahmood fair enough

harshilsharma63 avatar Oct 27 '22 05:10 harshilsharma63

/update-branch

sbishel avatar Nov 04 '22 15:11 sbishel

Testing I found a couple issues. Listed here in order of importance.

  1. Does not respect max file size setting in config. Screen Shot 2022-11-04 at 10 39 46 AM

  2. File name on download is truncated if name is truncated in card view and in some instances extension comes is different (jpg). Screen Shot 2022-11-04 at 10 35 21 AM

Screen Shot 2022-11-04 at 10 28 03 AM

  1. No status, nor completion message. Not sure when the upload or download completes.

  2. When adding more than 2 attachments it should wrap the attachment objects vertically, rather than scrolling a single row horizontally. Screen Shot 2022-11-04 at 10 18 36 AM

  3. Only single upload available, spec calls for multiple. This can be handled later.

  4. Thumbnails are all red?

  5. Missing Toolbars (On "Download" and displaying full file name. (see spec)

sbishel avatar Nov 04 '22 16:11 sbishel

Hey Rajat. I didn't have time to do a thorough code review today. But we did load this PR on https://boards-bugbash75.test.mattermost.cloud/ and did some testing against it. It works well with smaller files however with larger files it has some issues.

  1. Lack of feedback - Any file that takes more than a couple seconds to load shows no feed back to the user. Even a 10 mb file leaves the user wondering what is happening. Actually during bug bash most people thought it wasn't working at all and they navigated away prior to the file completing.

I would suggest looking at Channels and how the provide feedback with upload percentage. Screen Shot 2022-11-10 at 12 32 15 PM

  1. File over limit should be caught prior to upload - Although if you load a file larger than the system console setting it will eventually fail correctly. Users won't wait due to issue 1. Also, this should be checked prior to attempting the upload.

Channels has also already solved this problem. Screen Shot 2022-11-10 at 12 30 25 PM

I'm going to ping @harshilsharma63 for a review on this as well.

sbishel avatar Nov 10 '22 22:11 sbishel

@Rajat-Dabade Tested it with Scott earlier and it's really starting to come together!

Noticed the confirmation dialog expands to the full width of the card. Is that expected @asaadmahmood?

Screen Shot 2022-11-10 at 2 28 54 PM

wuwinson avatar Nov 10 '22 22:11 wuwinson

@wuwinson Nops, should be like this. This is from another issue, but the width should be capped at 512px.

https://www.figma.com/file/SaGJHxdJXCLnM67a6M62nq/4192---Drag-and-Drop-Manual-Revert?node-id=1%3A3&viewport=1511%2C930%2C1&t=yeuufYqZ56vPc9xd-11

asaadmahmood avatar Nov 11 '22 08:11 asaadmahmood

/update-branch

harshilsharma63 avatar Nov 21 '22 11:11 harshilsharma63

Functionality issues-

  1. Downloading attached files always results in them being downloaded with a .jpeg file extension.
  2. I shouldn't see the delete and add attachment options if I don't have edit access to the board.

UI issues I noticed-

  1. The file name tooltip is cut off for the first attachment (the leftmost one) Screenshot 2022-11-23 at 11 10 58 AM

  2. The color of the options menu icon (three vertical dots) and the download icon are different. For the options menu icon it's rgba(var(--center-channel-color-rgb), 0.56), and for download icons its #c1c1c1. The value for the options icon is the correct one and should be used for the download icon as well. Screenshot 2022-11-23 at 11 11 25 AM

  3. The options menu has a hover background but it's missing in the download icon. It should appear for both- hover

  4. The delete attachment confirmation dialog is having the full width of the card. It should be the same width as other confirmation dialogs, for example, the delete board confirmation dialog.

harshilsharma63 avatar Nov 23 '22 08:11 harshilsharma63

Awesome work @Rajat-Dabade ! Works really well and all minor fixes have been done. LGTM!

harshilsharma63 avatar Nov 23 '22 13:11 harshilsharma63

/update-branch

sbishel avatar Nov 23 '22 17:11 sbishel

/update-branch

sbishel avatar Nov 23 '22 18:11 sbishel