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

Improve reply popup after thread update #4923

Merged
merged 21 commits into from
Nov 5, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
1f0cbcf
Avoid thread root id when using reply command
iProdigy Oct 25, 2023
0dc6b84
Show correct reply context
iProdigy Oct 28, 2023
85de2f6
Reply to correct parent
iProdigy Oct 28, 2023
598c861
refactor: use threads getter
iProdigy Oct 29, 2023
629cb05
chore: use getter in rest of IrcMessageHandler
iProdigy Oct 29, 2023
c96dfac
refactor: use MessagePtr alias
iProdigy Oct 29, 2023
8dffbb6
chore: reformat
iProdigy Oct 29, 2023
c86a5e2
chore: update changelog
iProdigy Oct 29, 2023
b9cca1e
fix: avoid illegal access when direct parent is not loaded
iProdigy Oct 31, 2023
2bf88b4
feat: add menu option to reply to root thread message
iProdigy Oct 31, 2023
e8acab5
Merge branch 'master' into fix/thread-ui-update
iProdigy Nov 1, 2023
5ae2f2f
chore(LoggingChannel): reformat
iProdigy Nov 1, 2023
839ff49
fix: use correct message reference
iProdigy Nov 1, 2023
46715de
chore: update changelog
iProdigy Nov 1, 2023
59e580c
Merge branch 'master' into fix/thread-ui-update
iProdigy Nov 3, 2023
b63918d
Directly call builder.setParent if message is a reply to the root thread
pajlada Nov 5, 2023
827cc2f
Directly call builder.setParent when looking through channel threads,
pajlada Nov 5, 2023
38b74ce
Directly call builder.setParent if we have to iterate through messages.
pajlada Nov 5, 2023
d319503
Merge branch 'master' into fix/thread-ui-update
pajlada Nov 5, 2023
6745c5e
Merge remote-tracking branch 'iprodigy/fix/thread-ui-update' into fix…
pajlada Nov 5, 2023
a35a43d
Apply same setParent changes to addMessage
pajlada Nov 5, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
- Minor: The account switcher is now styled to match your theme. (#4817)
- Minor: Add an invisible resize handle to the bottom of frameless user info popups and reply thread popups. (#4795)
- Minor: The installer now checks for the VC Runtime version and shows more info when it's outdated. (#4847)
- Minor: Add menu actions to reply directly to a message or the original thread root. (#4923)
- Minor: The `/reply` command now replies to the latest message of the user. (#4919)
- Bugfix: Fixed an issue where certain emojis did not send to Twitch chat correctly. (#4840)
- Bugfix: Fixed capitalized channel names in log inclusion list not being logged. (#4848)
Expand All @@ -29,6 +30,7 @@
- Bugfix: Fixed headers of tables in the settings switching to bold text when selected. (#4913)
- Bugfix: Fixed tooltips appearing too large and/or away from the cursor. (#4920)
- Bugfix: Fixed a crash when clicking `More messages below` button in a usercard and closing it quickly. (#4933)
- Bugfix: Fixed thread popup window missing messages for nested threads. (#4923)
- Dev: Change clang-format from v14 to v16. (#4929)
- Dev: Fixed UTF16 encoding of `modes` file for the installer. (#4791)
- Dev: Temporarily disable High DPI scaling on Qt6 builds on Windows. (#4767)
Expand Down
5 changes: 3 additions & 2 deletions src/messages/Message.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,8 @@ enum class MessageFlag : int64_t {
};
using MessageFlags = FlagsEnum<MessageFlag>;

struct Message;
using MessagePtr = std::shared_ptr<const Message>;
struct Message {
Message();
~Message();
Expand Down Expand Up @@ -88,12 +90,11 @@ struct Message {
// the reply thread will be cleaned up by the TwitchChannel.
// The root of the thread does not have replyThread set.
std::shared_ptr<MessageThread> replyThread;
MessagePtr replyParent;
uint32_t count = 1;
std::vector<std::unique_ptr<MessageElement>> elements;

ScrollbarHighlight getScrollBarHighlight() const;
};

using MessagePtr = std::shared_ptr<const Message>;

} // namespace chatterino
138 changes: 107 additions & 31 deletions src/providers/twitch/IrcMessageHandler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -243,10 +243,12 @@ void populateReply(TwitchChannel *channel, Communi::IrcMessage *message,
TwitchMessageBuilder &builder)
{
const auto &tags = message->tags();
if (const auto it = tags.find("reply-parent-msg-id"); it != tags.end())
if (const auto it = tags.find("reply-thread-parent-msg-id");
it != tags.end())
{
const QString replyID = it.value().toString();
auto threadIt = channel->threads().find(replyID);
std::shared_ptr<MessageThread> rootThread;
if (threadIt != channel->threads().end())
{
auto owned = threadIt->second.lock();
Expand All @@ -256,43 +258,80 @@ void populateReply(TwitchChannel *channel, Communi::IrcMessage *message,
updateReplyParticipatedStatus(tags, message->nick(), builder,
owned, false);
builder.setThread(owned);
return;
rootThread = owned;
}
}

MessagePtr foundMessage;

// Thread does not yet exist, find root reply and create thread.
// Linear search is justified by the infrequent use of replies
for (const auto &otherMsg : otherLoaded)
if (!rootThread)
{
if (otherMsg->id == replyID)
MessagePtr foundMessage;

// Thread does not yet exist, find root reply and create thread.
// Linear search is justified by the infrequent use of replies
for (const auto &otherMsg : otherLoaded)
{
// Found root reply message
foundMessage = otherMsg;
break;
if (otherMsg->id == replyID)
{
// Found root reply message
foundMessage = otherMsg;
break;
}
}
}

if (!foundMessage)
{
// We didn't find the reply root message in the otherLoaded messages
// which are typically the already-parsed recent messages from the
// Recent Messages API. We could have a really old message that
// still exists being replied to, so check for that here.
foundMessage = channel->findMessage(replyID);
if (!foundMessage)
{
// We didn't find the reply root message in the otherLoaded messages
// which are typically the already-parsed recent messages from the
// Recent Messages API. We could have a really old message that
// still exists being replied to, so check for that here.
foundMessage = channel->findMessage(replyID);
}

if (foundMessage)
{
std::shared_ptr<MessageThread> newThread =
std::make_shared<MessageThread>(foundMessage);
updateReplyParticipatedStatus(tags, message->nick(), builder,
newThread, true);

builder.setThread(newThread);
rootThread = newThread;
// Store weak reference to thread in channel
channel->addReplyThread(newThread);
}
}

if (foundMessage)
if (const auto parentIt = tags.find("reply-parent-msg-id");
parentIt != tags.end())
{
std::shared_ptr<MessageThread> newThread =
std::make_shared<MessageThread>(foundMessage);
updateReplyParticipatedStatus(tags, message->nick(), builder,
newThread, true);

builder.setThread(newThread);
// Store weak reference to thread in channel
channel->addReplyThread(newThread);
const QString parentID = parentIt.value().toString();
if (replyID == parentID)
{
if (rootThread)
{
builder.setParent(rootThread->root());
}
}
else
{
auto parentThreadIt = channel->threads().find(parentID);
if (parentThreadIt != channel->threads().end())
{
auto thread = parentThreadIt->second.lock();
if (thread)
{
builder.setParent(thread->root());
}
}
else
{
auto parent = channel->findMessage(parentID);
if (parent)
{
builder.setParent(parent);
}
}
}
}
}
}
Expand Down Expand Up @@ -1283,17 +1322,20 @@ void IrcMessageHandler::addMessage(Communi::IrcMessage *message,
TwitchMessageBuilder builder(chan.get(), message, args, content, isAction);
builder.setMessageOffset(messageOffset);

if (const auto it = tags.find("reply-parent-msg-id"); it != tags.end())
if (const auto it = tags.find("reply-thread-parent-msg-id");
it != tags.end())
{
const QString replyID = it.value().toString();
auto threadIt = channel->threads_.find(replyID);
if (threadIt != channel->threads_.end() && !threadIt->second.expired())
auto threadIt = channel->threads().find(replyID);
std::shared_ptr<MessageThread> rootThread;
if (threadIt != channel->threads().end() && !threadIt->second.expired())
{
// Thread already exists (has a reply)
auto thread = threadIt->second.lock();
updateReplyParticipatedStatus(tags, message->nick(), builder,
thread, false);
builder.setThread(thread);
rootThread = thread;
}
else
{
Expand All @@ -1307,10 +1349,44 @@ void IrcMessageHandler::addMessage(Communi::IrcMessage *message,
newThread, true);

builder.setThread(newThread);
rootThread = newThread;
// Store weak reference to thread in channel
channel->addReplyThread(newThread);
}
}

if (const auto parentIt = tags.find("reply-parent-msg-id");
parentIt != tags.end())
{
const QString parentID = parentIt.value().toString();
if (replyID == parentID)
{
if (rootThread)
{
builder.setParent(rootThread->root());
}
}
else
{
auto parentThreadIt = channel->threads().find(parentID);
if (parentThreadIt != channel->threads().end())
{
auto thread = parentThreadIt->second.lock();
if (thread)
{
builder.setParent(thread->root());
}
}
else
{
auto parent = channel->findMessage(parentID);
if (parent)
{
builder.setParent(parent);
}
}
}
}
}

if (isSub || !builder.isIgnored())
Expand Down
18 changes: 16 additions & 2 deletions src/providers/twitch/TwitchMessageBuilder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -623,15 +623,24 @@ void TwitchMessageBuilder::parseThread()
{
// set references
this->message().replyThread = this->thread_;
this->message().replyParent = this->parent_;
this->thread_->addToThread(this->weakOf());

// enable reply flag
this->message().flags.set(MessageFlag::ReplyMessage);

const auto &threadRoot = this->thread_->root();
MessagePtr threadRoot;
if (!this->parent_)
{
threadRoot = this->thread_->root();
}
else
{
threadRoot = this->parent_;
}

QString usernameText = SharedMessageBuilder::stylizeUsername(
threadRoot->loginName, *threadRoot.get());
threadRoot->loginName, *threadRoot);

this->emplace<ReplyCurveElement>();

Expand Down Expand Up @@ -1811,6 +1820,11 @@ void TwitchMessageBuilder::setThread(std::shared_ptr<MessageThread> thread)
this->thread_ = std::move(thread);
}

void TwitchMessageBuilder::setParent(MessagePtr parent)
{
this->parent_ = std::move(parent);
}

void TwitchMessageBuilder::setMessageOffset(int offset)
{
this->messageOffset_ = offset;
Expand Down
2 changes: 2 additions & 0 deletions src/providers/twitch/TwitchMessageBuilder.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ class TwitchMessageBuilder : public SharedMessageBuilder
MessagePtr build() override;

void setThread(std::shared_ptr<MessageThread> thread);
void setParent(MessagePtr parent);
void setMessageOffset(int offset);

static void appendChannelPointRewardMessage(
Expand Down Expand Up @@ -131,6 +132,7 @@ class TwitchMessageBuilder : public SharedMessageBuilder
bool bitsStacked = false;
bool historicalMessage_ = false;
std::shared_ptr<MessageThread> thread_;
MessagePtr parent_;

/**
* Starting offset to be used on index-based operations on `originalMessage_`.
Expand Down
13 changes: 11 additions & 2 deletions src/singletons/helper/LoggingChannel.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -132,8 +132,17 @@ void LoggingChannel::addMessage(MessagePtr message)
qsizetype colonIndex = messageText.indexOf(':');
if (colonIndex != -1)
{
QString rootMessageChatter =
message->replyThread->root()->loginName;
QString rootMessageChatter;
if (message->replyParent)
{
rootMessageChatter = message->replyParent->loginName;
}
else
{
// we actually want to use 'reply-parent-user-login' tag here,
// but it's not worth storing just for this edge case
rootMessageChatter = message->replyThread->root()->loginName;
}
messageText.insert(colonIndex + 1, " @" + rootMessageChatter);
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/widgets/dialogs/ReplyThreadPopup.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ ReplyThreadPopup::ReplyThreadPopup(bool closeAutomatically, QWidget *parent,
void ReplyThreadPopup::setThread(std::shared_ptr<MessageThread> thread)
{
this->thread_ = std::move(thread);
this->ui_.replyInput->setReply(this->thread_);
this->ui_.replyInput->setReply(this->thread_->root());
this->addMessagesFromThread();
this->updateInputUI();

Expand Down
14 changes: 8 additions & 6 deletions src/widgets/helper/ChannelView.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2331,6 +2331,10 @@ void ChannelView::addMessageContextMenuItems(QMenu *menu,

if (messagePtr->replyThread != nullptr)
{
menu->addAction("Reply to &original thread", [this, &messagePtr] {
this->setInputReply(messagePtr->replyThread->root());
});

menu->addAction("View &thread", [this, &messagePtr] {
this->showReplyThreadPopup(messagePtr);
});
Expand Down Expand Up @@ -2871,19 +2875,17 @@ void ChannelView::setInputReply(const MessagePtr &message)
return;
}

auto thread = message->replyThread;

if (!thread)
if (!message->replyThread)
{
// Message did not already have a thread attached, try to find or create one
if (auto *tc =
dynamic_cast<TwitchChannel *>(this->underlyingChannel_.get()))
{
thread = tc->getOrCreateThread(message);
tc->getOrCreateThread(message);
}
else if (auto *tc = dynamic_cast<TwitchChannel *>(this->channel_.get()))
{
thread = tc->getOrCreateThread(message);
tc->getOrCreateThread(message);
}
else
{
Expand All @@ -2894,7 +2896,7 @@ void ChannelView::setInputReply(const MessagePtr &message)
}
}

this->split_->setInputReply(thread);
this->split_->setInputReply(message);
}

void ChannelView::showReplyThreadPopup(const MessagePtr &message)
Expand Down
2 changes: 1 addition & 1 deletion src/widgets/splits/Split.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1542,7 +1542,7 @@ void Split::drag()
stopDraggingSplit();
}

void Split::setInputReply(const std::shared_ptr<MessageThread> &reply)
void Split::setInputReply(const MessagePtr &reply)
{
this->input_->setReply(reply);
}
Expand Down
3 changes: 1 addition & 2 deletions src/widgets/splits/Split.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
namespace chatterino {

class ChannelView;
class MessageThread;
class SplitHeader;
class SplitInput;
class SplitContainer;
Expand Down Expand Up @@ -75,7 +74,7 @@ class Split : public BaseWidget

void setContainer(SplitContainer *container);

void setInputReply(const std::shared_ptr<MessageThread> &reply);
void setInputReply(const MessagePtr &reply);

static pajlada::Signals::Signal<Qt::KeyboardModifiers>
modifierStatusChanged;
Expand Down
Loading
Loading