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

FEAT(client): Add shortcut to send plain text messages #5744

Merged
merged 1 commit into from
Jul 25, 2022

Conversation

Snowknight26
Copy link
Contributor

Add the ability to press Ctrl+Enter to send a plain text message while
preserving line breaks. This adds an easy way of preserving messages
with line breaks without having to resort to using Markdown.

Implements #5707

Checks

src/mumble/MainWindow.cpp Outdated Show resolved Hide resolved
src/mumble/MainWindow.cpp Outdated Show resolved Hide resolved
src/mumble/MainWindow.h Outdated Show resolved Hide resolved
@Krzmbrzl Krzmbrzl added client feature-request This issue or PR deals with a new feature labels Jul 22, 2022
@Krzmbrzl Krzmbrzl linked an issue Jul 22, 2022 that may be closed by this pull request
@Snowknight26
Copy link
Contributor Author

This might be outside of the scope of this feature, but when sending a message with multiple simultaneous spaces, the multiple spaces are rendered as one space, regardless of whether the existing Markdown path is taken or this new 'plain text' one. This happens because the log is a QTextDocument and renders the string as HTML. Is that something that should be tackled?

@Krzmbrzl
Copy link
Member

Yes I think it should be tackled. After all I think that a user would expect a plain-text message to really be sent as-is - including all spaces.

I think the issue would be easily solved by replacing all spaces by non-breakable spaces. We'd have to check though if that can somehow mess with line breaking/wrapping

@Snowknight26
Copy link
Contributor Author

Preliminary testing with replacing a space with  , which I honestly thought would not have worked due to the nature of non-breaking spaces being non-breaking, seems to work.

Sent the following via the new plain text method:

line1start                                                                                                                          line1end
line2start                                                                                   line2end
line3start                                    line3end

Depending on the window size, this is how it appeared:
image
image
image
image

The only issue I can possibly see is the fact that with non-breaking spaces, if a line too long to be displayed without wrapping, when it wraps it also moves to the next line (notice To Root: on its own line in the 2nd-4th screenshots).

I tried to get around this by wrapping each plain text message and using some CSS to prevent whitespace collapsing using both <span style='white-space: pre'>%1</span> and <span style='white-space: pre-wrap'>%1</span>, and while both technically fixed that, it introduced other issues, including one where, upon wrapping, it seemed like some spaces were still collapsed, though only visually. Copying the message from the log correctly preserved the spaces.
image

If we're ok with the non-breaking space trick, I'll commit that.

@Krzmbrzl
Copy link
Member

If we're ok with the non-breaking space trick, I'll commit that.

Yeah, I think that's good enough ^^
And I wouldn't worry about the line break after the To ... part of the message. I think that's not too bad 🤷

Add the ability to press Ctrl+Enter to send a plain text message while
preserving line breaks. This adds an easy way of preserving messages
with line breaks without having to resort to using Markdown.

Implements mumble-voip#5707
@Krzmbrzl Krzmbrzl merged commit c87fb2b into mumble-voip:master Jul 25, 2022
@Krzmbrzl
Copy link
Member

Thank you very much for this contribution 👍

@Snowknight26 Snowknight26 deleted the send-as-plain-text branch July 25, 2022 12:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
client feature-request This issue or PR deals with a new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Please add an option to send plain text
2 participants