humhub / mail

A private messaging system for direct communication.
https://marketplace.humhub.com/module/mail
29 stars 50 forks source link

Possibility to attach files to a message entry #324

Closed marc-farre closed 1 year ago

marc-farre commented 1 year ago

I need to add a button to easily attach files to the Message Entry (e.g. audio messages or photos from the smartphone): image

@luke- do you prefer a PR where AbstractMessageEntry extends ContentActiveRecord instead of ActiveRecord and where I'll add this upload button, or should I create new module (e.g. messenger-advanced) for that? Thanks!

luke- commented 1 year ago

@funkycram In general I would be open for a PR in the official module. @Eladnarlea What do you think?

Eladnarlea commented 1 year ago

@luke- @funkycram yes, lets do this. It is also part of a future design optimisation, therefore it would come in handy if it was implemented already and the users could get used to this new button before the new look is introduced.

marc-farre commented 1 year ago

Thanks! So I'll do that. I will change AbstractMessageEntry to extend ContentActiveRecord.

Or perhaps it should extend ContentAddonActiveRecord? In this case I need to add object_model which will always have for value \humhub\modules\mail\models\Message. And change message_id to object_id.

What is your suggestion @luke- ?

luke- commented 1 year ago

@funkycram Oh, I would avoid that messages become content. Can't files already be attached to messages without being that model change?

marc-farre commented 1 year ago

Indeed, sorry, I had in mind that you could only attach files to content, maybe because of content_id in the file table...

What do you think about this?

image

image

Eladnarlea commented 1 year ago

Thank you @funkycram for starting to implement and design this!

Here are some suggestions:

image

I wouldn't recommend making the files button color predominate over the "send" button hierarchically, since the first and most important action is sending the text message (attaching files is optional). Also if the field is inactive I would put it in default state and only "activate" it when someone is actively typing a message. We might consider making them both look the same when inactive, and when active, the submit button becomes the dominant color and the file button remains secondary. Also: how do you propose it to look like in responsive mode?

For the text message itself: Do you suggest to display the files content type as shown in "clovnen.jpg"? I think it is a good suggestion although I would consider to only show it this way if the files type is not a picture (jpeg/jpg/maybe even png) but an actual file, For instance xsl/otf....

image

I guess your design is based on the "clean theme"?

For this one I also suggest making the "attach files" button less dominant as I think it might be too prominent. It is still optional to attach files. This could be achieved through using a "word-only" or "icon-only" button as this is a secondary button and/or change the color to a secondary color. :) EDIT: The background of the file in preview is streched over the full length of the section. Should we limit it to the length of the file name? (if more files are attached at the same time they could be displayed next to each other)

What do you think about it @funkycram @luke- ?

marc-farre commented 1 year ago

Thanks @Eladnarlea for the suggestions. In fact, I was thinking about having the same design as the comment one: image So yes, on my screenshot, the colors where not good (too quick test!). I think it's nice if we can have a uniformed design.

Yes, the screenshots are with the Clean Theme, I should have done them with the Community theme.

About the files preview, I don't want to create a new custom widget, I would like to use either the FilePreview::widget or ShowFiles::widget (with image preview). @luke- have you got a suggestion to easily meet @Eladnarlea recommandations?

For the "Attach files" button, would it be ok only changing the color? image

spoorun commented 1 year ago

Perhaps using a 'paperclip' icon would be more appropriate in this context? Given the user is not uploading the files to the Humhub system (as the upload button implies), but merely attaching them to the message...

marc-farre commented 1 year ago

Thanks @spoorun for the suggestion. Personally, I prefer uniformisation and avoiding having specific views for each different pages. I think conversation entries should look as close as possible to comments: image

Update: see https://github.com/humhub/humhub/issues/6298

@luke- we have some specific CSS for the comment module: https://github.com/humhub/humhub/blob/99bd1fddce5b4654d92d4513a6013990b6b0dfbb/static/less/comment.less#L104-L152 What do you think about having more generic classes, not related to the comment module, which could be used by the mail module (and potentially other modules)? In this case, have you got class name suggestions? E.g. comment_create becomes message_create, etc. And then I create a new message.less file to move the CSS part into.

marc-farre commented 1 year ago

@Eladnarlea the screenshots where done with Humhub 1.14.0 but now I'm using the develop branch (1.15.0 Unreleased), I see that the file name is not displayed anymore for images.

Test with 2 files (image and PDF file).

Comment: image

Message: image

Do you think I should zoom the image the same way it has been done on the comment?

Eladnarlea commented 1 year ago

Do you think I should zoom the image the same way it has been done on the comment?

@funkycram when I post a picture and a PDF at the same time it looks like this:

Screenshot 2023-05-08 at 8 15 23 AM

I think it is cool to have the image on full width although I don't know if I used the zoom-in feature. On top of that I haven't decided yet, which "pdf-preview" I like better. I think I would make it more like your pdf preview but with the text aligned to the left without the padding-left. I would probably also change the red color of the Icon and make the text #55555 instead of #21a1b3. What do you reckon @luke- @funkycram ?

Eladnarlea commented 1 year ago

For the "Attach files" button, would it be ok only changing the color?

from my side okay. Although I would make it slimmer probably, like this:

Screenshot 2023-05-08 at 8 30 16 AM

@funkycram what do you think about it?

marc-farre commented 1 year ago

OK, thanks @Eladnarlea for the suggestions. I'll send a PR and screenshots.

But before, I need an answer about this new issue: https://github.com/humhub/humhub/issues/6298

@luke- what do you think about it? If it's a bad idea, I'll close the issue and put the CSS in the mail module. Thanks!

marc-farre commented 1 year ago

@Eladnarlea I've had a look at the full width for images and I cannot change it as the ShowFiles widget (from the file module) doesn't have any option to change this. See https://github.com/humhub/humhub/blob/18bbe35312b8b0ed9e131eff573f4d2c5fe20b4e/protected/humhub/modules/file/widgets/views/showFiles.php#L34-L34

About the upload button, it now looks the same as the Comment module.

Latest previews: image image

Is it OK for you? If not, we need to change the CSS on the Humhub repository (as it concerns also the Comment module), not in this module and we need a separated issue and PR.

Eladnarlea commented 1 year ago

Is it OK for you? If not, we need to change the CSS on the Humhub repository (as it concerns also the Comment module), not in this module and we need a separated issue and PR.

I am not quite sure if I understand you right: do you want to change the design of the "mail/messenger module"-input field to look alike the comment section input field? We have implemented the new design to move towards a renewed design approach and modernised look. That's also the reason why the comment section input field and the messenger-module-input-field don't look alike..

marc-farre commented 1 year ago

@Eladnarlea

do you want to change the design of the "mail/messenger module"-input field to look alike the comment section input field?

Yes, this is done here: https://github.com/humhub/humhub/issues/6298

We have implemented the new design to move towards a renewed design approach and modernised look

Where can I found this new design? On my Humhub instance (develop branch), the Comment input looks like this: image

Is this new design not on the develop branch, i.e. https://github.com/humhub/humhub/blob/e9f72e871cbad0e47f99fb2e6782f615afb5b5c8/static/less/form.less#L346C4-L411 ?

Eladnarlea commented 1 year ago

The new design is the one that was introduced with the update of the new messenger module:

Screenshot 2023-05-09 at 2 21 24 PM

Is this new design not on the develop branch, i.e. https://github.com/humhub/humhub/blob/e9f72e871cbad0e47f99fb2e6782f615afb5b5c8/static/less/form.less#L346C4-L411 ?

I am sorry but I can not answer this question. @luke- ?

marc-farre commented 1 year ago

@luke- PR https://github.com/humhub/mail/pull/330

@Eladnarlea thanks for this information. I'm not aware of this new design and don't know where to find it.

@luke- maybe you know if this new design cancels the work done for https://github.com/humhub/humhub/issues/6298 (PR https://github.com/humhub/humhub/pull/6303)

luke- commented 1 year ago

@funkycram We plan (November '23 or March '24 Release) to integrate the "Send" button and the "Attach Files" buttons as a bottom line in the RichText Editor. e.g.

Draft Focussed Editor: (ToDo: Here we need to add a drop down for all file handlers) image

Draft UnFocussed Editor: image

This editor should be used as consistently as possible (e.g. Posts, Comments, Messages).

Until then, we can render the Submit & Files button of the Messages module (as suggested by you) outside of the RichText Editor widget, even if a later release will integrate these buttons directly in the editor.

marc-farre commented 1 year ago

Thanks for the precision! I like very much the new design! It has given me the idea to add an icon to the handlers:

image

Commit https://github.com/humhub/humhub/pull/6302/commits/d389526dac01c14868158d1ba226864c2a13fa2b

luke- commented 1 year ago

@funkycram It would be good if we could provide a property for this in the handler class. Then we would be more flexible in the future. Can you check this out?

Eladnarlea commented 1 year ago

Thanks for the precision! I like very much the new design! It has given me the idea to add an icon to the handlers:

image

Commit humhub/humhub@d389526

@funkycram right on! You see where this is going... :) In the future design we can display the most used / important icons in the text field already so the users can easily use these features. For the differentiation between the camera and video icon: Do you think it is necessary to use both, or do you think only using the camera icon could be enough, as most of the people have videos and photos together in their camera roll on their mobile phone/computer? -> less space is always good especially on mobile devices..

marc-farre commented 1 year ago

@luke-

provide a property for this in the handler class

Yes, I've created the abstract class UploadFileHandler (I've got the idea because we already have the DownloadFileHandler class): https://github.com/humhub/humhub/commit/c1f398657844228d8d3ec81b358dea0c0b085961

marc-farre commented 1 year ago

@Eladnarlea

Do you think it is necessary to use both

Yes, because on mobile it will open the camera in photo or video mode, but not both at the same time. E.g. you select "Video": the phone will show you already taken videos (but no photos) and will let you take a new video (but no photo).

Eladnarlea commented 1 year ago

@funkycram ah two things:

  1. you plan to integrate the option to take photos right inside the humhub comment/messenger? that would be cool.

  2. so when you open the camera icon you get to the camera of your phone and have the gallery attached down in the left bottom corner? And you can't switch between photo and video even though you're in your own camera modus? Sorry for all the questions, I just want to get it right! :)

marc-farre commented 1 year ago

@Eladnarlea Yes for point 1 and 2. This is how it looks like:

audio resized

image resized

video resized

spoorun commented 1 year ago

Looking at a variety of other messaging apps (Insta messenger, WhatsApp, Messages, Messenger (SMS), Skype, Telegram, Wechat, Signal) :

Having looked at the mock-ups, just to clarify, the sender would see the streamlined 'comment' field, with rich text entry bar, and the following icons:

marc-farre commented 1 year ago

@spoorun I agree with you. For Humhub 1.15 release, it will be a dropdown for only audio, camera and video. But later, this will be reviewed as shown on https://github.com/humhub/mail/issues/324#issuecomment-1540540397

marc-farre commented 1 year ago

@luke- Summary of this new feature:

Required closed issue and merged PR into core develop:

Eladnarlea commented 1 year ago

@spoorun

Thank you very much for your research! It is good to already have everything listed, especially in comparison to other messenger services. :) I like the ideas to add location, contact and several other icons to the message entry field. We also have that in mind and we will try to find a solution that will align all our imaginations.

Having looked at the mock-ups, just to clarify, the sender would see the streamlined 'comment' field, with rich text entry bar, and the following icons:

As this mock-up is only a glimpse into what we are working on, I wouldn't finalise the choice of icons just yet. Let's discuss this when the topic becomes relevant.

Eladnarlea commented 1 year ago

Yes for point 1 and 2.

thank you for showing me @funkycram

marc-farre commented 1 year ago

@Eladnarlea On the screenshots, you cannot see the difference between videos and photos on the button image but when you click on it, depending on the choice made on Humhub, the camera starts in video or photo mode.

Eladnarlea commented 1 year ago

Do you also have to choose first wether you like to take a video or photo when you use whatsapp for instance? @funkycram

marc-farre commented 1 year ago

Do you also have to choose first wether you like to take a video or photo when you use whatsapp for instance? @funkycram

@Eladnarlea we cannot compare with a native app because we are restricted by the mobile browser capacities. On browsers, we need to distinguish audio, images and videos. On native apps, we can have a single button for both images and videos. So maybe to could be improved on Humhub app.

luke- commented 1 year ago

@funkycram Maybe it's possible to specify multiple types e.g. audio/*, video/*? Then we could start for the time being this way and decide later if it's necessary to split into two different handlers. also if necessary before via configuration.

marc-farre commented 1 year ago

Maybe it's possible to specify multiple types e.g. audio/*, video/*?

Tested on Chrome (latest) Android 12: both images and videos are displayed in the file browser, but when you click on the camera button, you can only take photos. You cannot switch to video.

So the only way, in this Chrome/Android configuration (which is most common), if you want to allow taking a video directly, is to separate the images and videos buttons.

luke- commented 1 year ago

Ok, then lets go with seperate options

marc-farre commented 1 year ago

@luke- Maybe we could merge this PR https://github.com/humhub/mail/pull/330 now that Humhub 1.15 beta has been released?

luke- commented 1 year ago

@marc-farre Just merged into master. But I had some problems with the Stylesheet changes. Can you please check?

marc-farre commented 1 year ago

@luke- I think this is because in the mean time, the Send button has been integrated in the RichText Editor. see your comment https://github.com/humhub/mail/issues/324#issuecomment-1540540397

PR which fixes the problem: https://github.com/humhub/mail/pull/350

image