Feature/3949/allow editing before upload #3988

marcoambrosini · 2020-08-03T13:42:56Z

Follow-up:

Prevent duplicate uploads
Upload indicator should be overlaid on image, and just vanish when it’s uploaded
Re-drop files on modal

Signed-off-by: Marco Ambrosini <marcoambrosini@pm.me>

danxuliu · 2020-08-12T02:15:20Z

src/components/MessagesList/MessagesGroup/Message/MessagePart/FilePreview.vue

-	/* Show a hover colour around the preview when navigating with the
-	 * keyboard through the file links (or hovering them with the mouse). */
-	&:hover,
-	&:focus,
-	&:active {
-		.preview {
-			background-color: var(--color-background-hover);
+	border-radius: 16px;

-			/* Trick to keep the same position while adding a padding to show
-			 * the background. */
-			box-sizing: content-box !important;
-			padding: 10px;
-			margin: -10px;
-		}
+	&:hover,
+	&:focus {
+		background-color: var(--color-background-hover);
+		/* Trick to keep the same position while adding a padding to show
+			* the background. */
+		box-sizing: content-box !important;


While this looks good in the upload editor in my opinion the look in the messages list has regressed. Therefore I would suggest to have both the previous style and the new one, and use one or the other depending on whether the upload-editor class is set or not.

What do you think has regressed in particular? Before there was an unneeded hover feedback on the preview itself that partially covered the sender's username.

there was an unneeded hover feedback on the preview itself

I think that the hover feedback should be there, as it shows the user that she can interact with the file. The problem is that the whole message is "highlighted" on hover when showing the reply button, and that hides the specific feedback for the file. Anyway, I would be fine with removing the hover feedback on the file due to that overlap (but I would prefer to find a better solution nevertheless).

that partially covered the sender's username

I know, it has been haunting me since I added it :-P

However, even if it covers the sender's username I prefer that small, focused frame on the file:

Rather than highlighting the whole message body (even if it actually matches the real clickable area):

For me the previous version feels and looks much better when navigating with the keyboard. Also the change is not directly related to editing the upload, so in any case I would address it in a separate pull request too.

So this is the current behavior (before this pr):

So you get the big hover feedback of the message but no clickability unless you hover the preview, and since the hover color is the same, you get the hover feedback of the preview only through unwanted stuff:

overflow from the message on the left;

invading the name on top;

The keyboard navigation is a bug with how messages hover feedback is implemented. For a reference of how it should really look refer to the mouse navigation feedback. Plus, I think that a bigger click area is generally better, since that space is unused anyway... I implemented the change in this pr because I'm adapting the filepreview component.

src/components/MessagesList/MessagesGroup/Message/MessagePart/FilePreview.vue

src/components/UploadEditor.vue

marcoambrosini

Form the design call:

Aspect ratio of images in uploader squished
"+" Button needs margin like the previews
Dismiss → Cancel, and Cancel button to left
Make sure "Remove" button is tabbable and also has an aria-label (including the filename)
Add aria-label to "+" Button (Added a part about the remove button accesibility, make sure they are labeled also incl the filename, not all just "Remove" :)

Signed-off-by: Marco Ambrosini <marcoambrosini@pm.me>

nickvergessen · 2020-08-17T19:08:55Z

Follow up:

Ctrl+V with an copied image works on the first try, but on follow up pastes, it just replaces the previous image.
When the dialog is open, drag+drop does not work anymore. Would be nice to be able to still drag+drop on top of the modal to add more images to the process

nickvergessen · 2020-08-17T19:14:47Z

src/components/MessagesList/MessagesGroup/Message/MessagePart/FilePreview.vue

-		rel="noopener noreferrer"
-		@click="showPreview">
-		<img v-if="!isLoading && !failed"
+	<file-preview v-bind="filePreview"


This file really bothers me. The shared part is actually so little, that it should just be 2 maintainable subcomponents instead.

marcoambrosini requested review from danxuliu and nickvergessen August 3, 2020 13:51

marcoambrosini self-assigned this Aug 3, 2020

marcoambrosini added 2. developing feature: frontend 🖌️ "Web UI" client enhancement labels Aug 3, 2020

marcoambrosini force-pushed the feature/3949/allow-editing-before-upload branch from 8291ec3 to 652bfe0 Compare August 5, 2020 13:49

marcoambrosini marked this pull request as ready for review August 5, 2020 13:51

marcoambrosini force-pushed the feature/3949/allow-editing-before-upload branch 3 times, most recently from ca1e3a0 to 4af73ed Compare August 5, 2020 15:40

nickvergessen added this to the 💚 Next Major (20) milestone Aug 5, 2020

marcoambrosini added 3. to review 2. developing and removed 2. developing 3. to review labels Aug 5, 2020

marcoambrosini added 7 commits August 12, 2020 00:06

Store current upload id

722fe35

Signed-off-by: Marco Ambrosini <marcoambrosini@pm.me>

Store visibility of the upload editor

6ef9611

Signed-off-by: Marco Ambrosini <marcoambrosini@pm.me>

Add uploadeditor component

661348e

Signed-off-by: Marco Ambrosini <marcoambrosini@pm.me>

Create initialiseduploads store entry and split uploadfiles logic

07fee78

Signed-off-by: Marco Ambrosini <marcoambrosini@pm.me>

Add ability to add more files to the current upload

551305d

Signed-off-by: Marco Ambrosini <marcoambrosini@pm.me>

Make the filepreview component compatible with upload editing

d116643

Signed-off-by: Marco Ambrosini <marcoambrosini@pm.me>

Add ability to remove files from selection

2915171

Signed-off-by: Marco Ambrosini <marcoambrosini@pm.me>

danxuliu force-pushed the feature/3949/allow-editing-before-upload branch from b017977 to 6c88f42 Compare August 12, 2020 02:11

danxuliu reviewed Aug 12, 2020

View reviewed changes

marcoambrosini commented Aug 12, 2020

View reviewed changes

marcoambrosini force-pushed the feature/3949/allow-editing-before-upload branch from 37fcaf0 to 7bdc55b Compare August 13, 2020 14:56

marcoambrosini force-pushed the feature/3949/allow-editing-before-upload branch from 7bdc55b to e6e2c43 Compare August 13, 2020 15:20

marcoambrosini requested a review from danxuliu August 13, 2020 15:20

marcoambrosini added 5 commits August 13, 2020 18:01

Show previews in the upload editor

ef09c4c

Signed-off-by: Marco Ambrosini <marcoambrosini@pm.me>

Move share logic to the uploadfiles method in the store

9e6e2d5

Signed-off-by: Marco Ambrosini <marcoambrosini@pm.me>

Style upload editor

c49ee85

Signed-off-by: Marco Ambrosini <marcoambrosini@pm.me>

Use unique random indexes for temporary messages

d738442

Signed-off-by: Marco Ambrosini <marcoambrosini@pm.me>

Make uploadeditor accessible

6653eb6

Signed-off-by: Marco Ambrosini <marcoambrosini@pm.me>

marcoambrosini force-pushed the feature/3949/allow-editing-before-upload branch from e6e2c43 to 6653eb6 Compare August 13, 2020 16:01

nickvergessen approved these changes Aug 17, 2020

View reviewed changes

nickvergessen reviewed Aug 17, 2020

View reviewed changes

nickvergessen merged commit 188f049 into master Aug 17, 2020

nickvergessen deleted the feature/3949/allow-editing-before-upload branch August 17, 2020 19:17

This was referenced Aug 18, 2020

Preview the item before the final "Share" is done to allow cancelation #3949

Closed

Attachment gets re-uploaded if you close/open the talk tab #3904

Closed

nickvergessen mentioned this pull request Aug 27, 2020

Add changelog for 10.0.0-beta.1 #4067

Merged

4 tasks

danxuliu mentioned this pull request Nov 5, 2020

Add upload editor in files sidebar mode #4535

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature/3949/allow editing before upload #3988

Feature/3949/allow editing before upload #3988

marcoambrosini commented Aug 3, 2020 •

edited

Loading

danxuliu Aug 12, 2020

marcoambrosini Aug 12, 2020

danxuliu Aug 12, 2020

marcoambrosini Aug 13, 2020

marcoambrosini left a comment •

edited

Loading

nickvergessen commented Aug 17, 2020

nickvergessen Aug 17, 2020

Feature/3949/allow editing before upload #3988

Feature/3949/allow editing before upload #3988

Conversation

marcoambrosini commented Aug 3, 2020 • edited Loading

danxuliu Aug 12, 2020

Choose a reason for hiding this comment

marcoambrosini Aug 12, 2020

Choose a reason for hiding this comment

danxuliu Aug 12, 2020

Choose a reason for hiding this comment

marcoambrosini Aug 13, 2020

Choose a reason for hiding this comment

marcoambrosini left a comment • edited Loading

Choose a reason for hiding this comment

nickvergessen commented Aug 17, 2020

nickvergessen Aug 17, 2020

Choose a reason for hiding this comment

marcoambrosini commented Aug 3, 2020 •

edited

Loading

marcoambrosini left a comment •

edited

Loading