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

Display message being replied to above input box #4350

Merged
merged 21 commits into from
Jul 14, 2024

Conversation

dnsge
Copy link
Contributor

@dnsge dnsge commented Jan 31, 2023

Pull request checklist:

  • CHANGELOG.md was updated, if applicable

Description

When you begin replying to a message (by right-clicking a message and selecting "reply" or clicking the reply button), the message being replied to will now display above the input box.

Example:
image

In the case of very long messages, the message will be truncated:

Expand for image image

The message displayed is uninteractable: you can't click on usernames, highlight text, or see emote tooltips for example. The reply button is hidden and you won't see inline replies if the message you're replying to is also a reply.

The very long message truncation is done using a fixed height limit. This limit was chosen to be big enough for most messages, but we could make it larger/smaller.

Fixes #3897

@@ -100,7 +101,7 @@ class SplitInput : public BaseWidget
/// Clears the input box, clears reply thread if inline replies are enabled
void clearInput();

protected:
private:
Copy link
Contributor Author

@dnsge dnsge Jan 31, 2023

Choose a reason for hiding this comment

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

This was changed at one point during my work on #3722 but was never changed back. Made its way through code review. I noticed it while working on this PR and figured I'd change it back.

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

clang-tidy made some suggestions

src/widgets/helper/MessageView.cpp Outdated Show resolved Hide resolved
src/widgets/helper/MessageView.cpp Outdated Show resolved Hide resolved
src/widgets/helper/MessageView.cpp Outdated Show resolved Hide resolved
src/widgets/helper/MessageView.cpp Outdated Show resolved Hide resolved
src/widgets/helper/MessageView.cpp Outdated Show resolved Hide resolved
src/widgets/helper/MessageView.cpp Outdated Show resolved Hide resolved
src/widgets/helper/MessageView.hpp Outdated Show resolved Hide resolved
src/widgets/helper/MessageView.hpp Outdated Show resolved Hide resolved
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

clang-tidy made some suggestions

src/widgets/helper/MessageView.hpp Outdated Show resolved Hide resolved
@sheybey
Copy link
Contributor

sheybey commented Feb 11, 2023

Looks fine on Windows for the most part, but the emote picker and close icons are broken. They still work as expected when I click on them. I'll try to debug that this afternoon.

Screenshot 2023-02-11 142614

@dnsge dnsge force-pushed the feat/show-message-replying-to branch from ba11ff4 to dcc6208 Compare December 10, 2023 04:16
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

clang-tidy made some suggestions

src/widgets/helper/MessageView.cpp Outdated Show resolved Hide resolved
src/widgets/helper/MessageView.cpp Outdated Show resolved Hide resolved
src/widgets/helper/MessageView.cpp Outdated Show resolved Hide resolved
src/widgets/helper/MessageView.cpp Outdated Show resolved Hide resolved
src/widgets/helper/MessageView.cpp Outdated Show resolved Hide resolved
src/widgets/helper/MessageView.hpp Outdated Show resolved Hide resolved
src/widgets/splits/SplitInput.cpp Show resolved Hide resolved
src/widgets/splits/SplitInput.cpp Outdated Show resolved Hide resolved
src/widgets/splits/SplitInput.cpp Show resolved Hide resolved
src/widgets/splits/SplitInput.hpp Outdated Show resolved Hide resolved
@dnsge
Copy link
Contributor Author

dnsge commented Dec 10, 2023

I've updated this PR with the latest changes from master. I'm happy with the current state, so I'm marking it as ready for review.

Something to decide: should showing the message being replied to be a setting? We could let the user toggle between what happens now (just the "replying to username" text) and this PR.

@dnsge dnsge marked this pull request as ready for review December 10, 2023 05:01
@Felanbird Felanbird added the Waiting for review PR bounced back to reviewer label Dec 30, 2023
@pajlada pajlada added this to the Post-2.5.0 milestone Mar 31, 2024
@pajlada
Copy link
Member

pajlada commented Jul 14, 2024

Looks like this now:
image

more like the "single line text" that's used in replies already

image

@pajlada pajlada removed the Waiting for review PR bounced back to reviewer label Jul 14, 2024
Copy link
Member

@pajlada pajlada left a comment

Choose a reason for hiding this comment

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

Looks good to me, I'm happy with how this looks.
Do you want to have a look before I merge this in @dnsge?

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

clang-tidy made some suggestions

src/widgets/splits/SplitInput.cpp Show resolved Hide resolved
src/widgets/splits/SplitInput.cpp Show resolved Hide resolved
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

clang-tidy made some suggestions

src/widgets/splits/SplitInput.cpp Show resolved Hide resolved
@dnsge
Copy link
Contributor Author

dnsge commented Jul 14, 2024

@pajlada looks good! Less noisy when we have highlights. Thanks for updating it.

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

clang-tidy made some suggestions

src/widgets/splits/SplitInput.cpp Show resolved Hide resolved
@Felanbird Felanbird merged commit 6b73bb5 into Chatterino:master Jul 14, 2024
17 checks passed
@dnsge dnsge deleted the feat/show-message-replying-to branch July 14, 2024 19:59
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.

Show message content in replying to ... textbox
4 participants