Skip to content

Comments

[NEW] Send too long messages as attachment#7676

Closed
lindoelio wants to merge 10 commits intoRocketChat:developfrom
lindoelio:attach-message-too-long
Closed

[NEW] Send too long messages as attachment#7676
lindoelio wants to merge 10 commits intoRocketChat:developfrom
lindoelio:attach-message-too-long

Conversation

@lindoelio
Copy link
Contributor

@RocketChat/core

Closes #7267

This PR include:

  • Settings for that RocketChat admins can to allow/deny the sending of too long messages as attachment. URL target: ../admin/Message.
    image

  • Offer for user to send too long messages as attachment.
    image
    image

@RocketChat RocketChat deleted a comment Aug 8, 2017
@RocketChat RocketChat deleted a comment Aug 8, 2017
@RocketChat RocketChat deleted a comment Aug 8, 2017
"Mentions": "Mentions",
"Mentions_default": "Mentions (default)",
"Message": "Message",
"Message_AllowAttachTooLongMessages": "Allow attach too long messages",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@geekgonecrazy any suggestion here?

Copy link
Contributor

@geekgonecrazy geekgonecrazy Sep 13, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Convert long messages to attachment would be a bit better. Would also change the i18n name.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep... This message appear for admins so that they allow/deny the converting of long messages to attachment. It sounds good: "Message_AllowConvertLongMessagesToAttachment"?

closeOnConfirm: true
}, () => {
// Transform message to attachment...
const contentType = 'application/vnd.openxmlformats-officedocument.wordprocessingml.document';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we have a simpler format here? Having it as plain/text would be way better for compatibility.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed... We need a more compatible format

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed. I'll to perform it.

}

fileUpload = function(filesToUpload) {
performFileUpload = function(file, fileName, fileDescription) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why the modifications here? Couldn't we use the default file upload?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gdelavald, We are using the default file upload here still. What I did was generalize part of original fileUpload function so that it could be more reusable. You can see that the fileUpload function have calling "performFileUpload" function too so perform the file upload core activity. :-)

@gdelavald
Copy link
Contributor

@lindoelio can you check the comments and make some changes? I really like this feature. 👍 for doing this.

@lindoelio lindoelio self-assigned this Sep 14, 2017
@engelgabriel engelgabriel temporarily deployed to rocket-chat-pr-7676 September 18, 2017 14:01 Inactive
@geekgonecrazy geekgonecrazy dismissed gdelavald’s stale review September 18, 2017 17:18

addressed these changes 👍

@gdelavald
Copy link
Contributor

Message box doesn't send message on enter, could you take a look at this issue?

@engelgabriel engelgabriel temporarily deployed to rocket-chat-pr-7676 October 25, 2017 17:27 Inactive
@engelgabriel engelgabriel temporarily deployed to rocket-chat-pr-7676 October 27, 2017 00:15 Inactive
@geekgonecrazy
Copy link
Contributor

@gdelavald did this last commit fix it for you?

@sampaiodiego sampaiodiego added this to the 0.75.0 milestone Jan 17, 2019
@marceloschmidt
Copy link
Member

Due to conflicts and non-existing fork anymore, I made a new PR: #13819

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants