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 "View Description" context action to channels #6526

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
30 changes: 30 additions & 0 deletions src/mumble/MainWindow.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2258,6 +2258,7 @@ void MainWindow::qmChannel_aboutToShow() {
qmChannel->addAction(qaChannelUnlinkAll);
qmChannel->addSeparator();
qmChannel->addAction(qaChannelCopyURL);
qmChannel->addAction(qaChannelDescriptionView);
qmChannel->addAction(qaChannelSendMessage);

// hiding the root is nonsense
Expand Down Expand Up @@ -2310,6 +2311,7 @@ void MainWindow::qmChannel_aboutToShow() {
if (c) {
qaChannelHide->setChecked(c->m_filterMode == ChannelFilterMode::HIDE);
qaChannelPin->setChecked(c->m_filterMode == ChannelFilterMode::PIN);
qaChannelDescriptionView->setEnabled(!c->qbaDescHash.isEmpty());
}

qaChannelAdd->setEnabled(add);
Expand Down Expand Up @@ -2531,6 +2533,34 @@ void MainWindow::on_qaChannelCopyURL_triggered() {
QClipboard::Clipboard);
}

void MainWindow::on_qaChannelDescriptionView_triggered() {
Channel *c = getContextMenuChannel();
// This has to be done here because UserModel could've set it.
cContextChannel.clear();

if (!c)
return;

Comment on lines +2540 to +2543
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if (!c)
return;
if (!c) {
return;
}

if (!c->qbaDescHash.isEmpty() && c->qsDesc.isEmpty()) {
c->qsDesc = QString::fromUtf8(Global::get().db->blob(c->qbaDescHash));
if (c->qsDesc.isEmpty()) {
pmModel->iChannelDescription = ~static_cast< int >(c->iId);
Copy link
Member

Choose a reason for hiding this comment

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

Just for the record, I think here the same issue with the root channel applies that Hartmnt has found elsewhere already.

MumbleProto::RequestBlob mprb;
mprb.add_channel_description(c->iId);
Global::get().sh->sendMessage(mprb);
return;
}
}

pmModel->seenComment(pmModel->index(c));

::TextMessage *texm = new ::TextMessage(this, tr("View description of channel %1").arg(c->qsName));

texm->rteMessage->setText(c->qsDesc, true);
texm->setAttribute(Qt::WA_DeleteOnClose, true);
texm->show();
}

/**
* This function updates the UI according to the permission of the user in the current channel.
* If possible the permissions are fetched from a cache. Otherwise they are requested by the server
Expand Down
1 change: 1 addition & 0 deletions src/mumble/MainWindow.h
Original file line number Diff line number Diff line change
Expand Up @@ -271,6 +271,7 @@ public slots:
void on_qaChannelHide_triggered();
void on_qaChannelPin_triggered();
void on_qaChannelCopyURL_triggered();
void on_qaChannelDescriptionView_triggered();
void on_qaAudioReset_triggered();
void on_qaAudioMute_triggered();
void on_qaAudioDeaf_triggered();
Expand Down
8 changes: 8 additions & 0 deletions src/mumble/MainWindow.ui
Original file line number Diff line number Diff line change
Expand Up @@ -919,6 +919,14 @@ the channel's context menu.</string>
<string>&amp;Pin When Filtering</string>
</property>
</action>
<action name="qaChannelDescriptionView">
<property name="text">
<string>Vie&amp;w Description</string>
</property>
<property name="toolTip">
<string>View description in editor</string>
</property>
</action>
<action name="qaUserCommentView">
<property name="text">
<string>Vie&amp;w Comment</string>
Expand Down
4 changes: 4 additions & 0 deletions src/mumble/UserModel.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1271,6 +1271,10 @@ void UserModel::setComment(Channel *c, const QString &comment) {
QToolTip::showText(QCursor::pos(), data(index(c, 0), Qt::ToolTipRole).toString(),
Global::get().mw->qtvUsers);
}
} else if (c->iId == static_cast< unsigned int >(~iChannelDescription)) {
iChannelDescription = -1;
Comment on lines +1274 to +1275
Copy link
Member

Choose a reason for hiding this comment

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

The whole casting here looks somewhat fishy. It is probably working as expected (and I realize you replicated the same mechanism as for the user comment).
I think this might theoretically break when reaching like 2 billion channels, which would be fine. But have considered edge cases here? root has channel id 0 which would be ~0 == -1 when considering signed integers.

time passes

Haha, ok guess what. This indeed breaks for root. I just tried it. If you set the description of the root channel, a message will popup every time the content is received from the server. (e.g. when reconnecting).
Sadly as of posting this I have no solid idea how to fix this.

Copy link
Contributor Author

@dexgs dexgs Sep 15, 2024

Choose a reason for hiding this comment

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

The best thing I can think of without refactoring receiving descriptions/comments from the server is to add an extra field to UserModel which is an enum indicating what the received description is for (popup vs. "view description" with the flexibility to support more cases in the future)

If this is acceptable, I can try implementing it soon

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the problem doesn't exist in ClientUser since users never get session id 0, but using the negation of the IDs to differentiate between popup and the "view description/comment" option is hacky in both cases

Copy link
Member

Choose a reason for hiding this comment

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

The extra field as an enum sounds good to me.

Copy link
Member

Choose a reason for hiding this comment

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

I am all for switching to a "proper" implementation instead of negating ids. I do not think anything will break if you remove the existing negation logic.

Global::get().mw->cContextChannel = c;
QTimer::singleShot(0, Global::get().mw, &MainWindow::on_qaChannelDescriptionView_triggered);
} else {
item->bCommentSeen = Global::get().db->seenComment(item->hash(), c->qbaDescHash);
newstate = item->bCommentSeen ? 2 : 1;
Expand Down
12 changes: 12 additions & 0 deletions src/mumble/mumble_ar.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7002,6 +7002,18 @@ Valid options are:
<source>Filtered channels and users</source>
<translation type="unfinished"></translation>
</message>
<message>
<source>Vie&amp;w Description</source>
<translation type="unfinished"></translation>
</message>
<message>
<source>View description in editor</source>
<translation type="unfinished"></translation>
</message>
<message>
<source>View description of channel %1</source>
<translation type="unfinished"></translation>
</message>
</context>
<context>
<name>Manual</name>
Expand Down
12 changes: 12 additions & 0 deletions src/mumble/mumble_bg.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6999,6 +6999,18 @@ Valid options are:
<source>Filtered channels and users</source>
<translation type="unfinished"></translation>
</message>
<message>
<source>Vie&amp;w Description</source>
<translation type="unfinished"></translation>
</message>
<message>
<source>View description in editor</source>
<translation type="unfinished"></translation>
</message>
<message>
<source>View description of channel %1</source>
<translation type="unfinished"></translation>
</message>
</context>
<context>
<name>Manual</name>
Expand Down
12 changes: 12 additions & 0 deletions src/mumble/mumble_br.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6998,6 +6998,18 @@ Valid options are:
<source>Filtered channels and users</source>
<translation type="unfinished"></translation>
</message>
<message>
<source>Vie&amp;w Description</source>
<translation type="unfinished"></translation>
</message>
<message>
<source>View description in editor</source>
<translation type="unfinished"></translation>
</message>
<message>
<source>View description of channel %1</source>
<translation type="unfinished"></translation>
</message>
</context>
<context>
<name>Manual</name>
Expand Down
12 changes: 12 additions & 0 deletions src/mumble/mumble_ca.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7152,6 +7152,18 @@ Les opcions vàlides són:
<source>Filtered channels and users</source>
<translation>Canals i usuaris filtrats</translation>
</message>
<message>
<source>Vie&amp;w Description</source>
<translation type="unfinished"></translation>
</message>
<message>
<source>View description in editor</source>
<translation type="unfinished"></translation>
</message>
<message>
<source>View description of channel %1</source>
<translation type="unfinished"></translation>
</message>
</context>
<context>
<name>Manual</name>
Expand Down
12 changes: 12 additions & 0 deletions src/mumble/mumble_cs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7058,6 +7058,18 @@ Valid options are:
<source>Filtered channels and users</source>
<translation type="unfinished"></translation>
</message>
<message>
<source>Vie&amp;w Description</source>
<translation type="unfinished"></translation>
</message>
<message>
<source>View description in editor</source>
<translation type="unfinished"></translation>
</message>
<message>
<source>View description of channel %1</source>
<translation type="unfinished"></translation>
</message>
</context>
<context>
<name>Manual</name>
Expand Down
12 changes: 12 additions & 0 deletions src/mumble/mumble_cy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7002,6 +7002,18 @@ Valid options are:
<source>Filtered channels and users</source>
<translation type="unfinished"></translation>
</message>
<message>
<source>Vie&amp;w Description</source>
<translation type="unfinished"></translation>
</message>
<message>
<source>View description in editor</source>
<translation type="unfinished"></translation>
</message>
<message>
<source>View description of channel %1</source>
<translation type="unfinished"></translation>
</message>
</context>
<context>
<name>Manual</name>
Expand Down
12 changes: 12 additions & 0 deletions src/mumble/mumble_da.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7054,6 +7054,18 @@ Valid options are:
<source>Filtered channels and users</source>
<translation type="unfinished"></translation>
</message>
<message>
<source>Vie&amp;w Description</source>
<translation type="unfinished"></translation>
</message>
<message>
<source>View description in editor</source>
<translation type="unfinished"></translation>
</message>
<message>
<source>View description of channel %1</source>
<translation type="unfinished"></translation>
</message>
</context>
<context>
<name>Manual</name>
Expand Down
12 changes: 12 additions & 0 deletions src/mumble/mumble_de.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7145,6 +7145,18 @@ Gültige Optionen sind:
<source>Filtered channels and users</source>
<translation>Gefilterte Kanäle und Benutzer</translation>
</message>
<message>
<source>Vie&amp;w Description</source>
<translation type="unfinished"></translation>
</message>
<message>
<source>View description in editor</source>
<translation type="unfinished"></translation>
</message>
<message>
<source>View description of channel %1</source>
<translation type="unfinished"></translation>
</message>
</context>
<context>
<name>Manual</name>
Expand Down
12 changes: 12 additions & 0 deletions src/mumble/mumble_el.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7152,6 +7152,18 @@ mumble://[&lt;username&gt;[:&lt;password&gt;]@]&lt;host&gt;[:&lt;port&gt;][/&lt;
<source>Filtered channels and users</source>
<translation type="unfinished"></translation>
</message>
<message>
<source>Vie&amp;w Description</source>
<translation type="unfinished"></translation>
</message>
<message>
<source>View description in editor</source>
<translation type="unfinished"></translation>
</message>
<message>
<source>View description of channel %1</source>
<translation type="unfinished"></translation>
</message>
</context>
<context>
<name>Manual</name>
Expand Down
12 changes: 12 additions & 0 deletions src/mumble/mumble_en.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6997,6 +6997,18 @@ Valid options are:
<source>Filtered channels and users</source>
<translation type="unfinished"></translation>
</message>
<message>
<source>Vie&amp;w Description</source>
<translation type="unfinished"></translation>
</message>
<message>
<source>View description in editor</source>
<translation type="unfinished"></translation>
</message>
<message>
<source>View description of channel %1</source>
<translation type="unfinished"></translation>
</message>
</context>
<context>
<name>Manual</name>
Expand Down
12 changes: 12 additions & 0 deletions src/mumble/mumble_en_GB.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7051,6 +7051,18 @@ Valid options are:
<source>Filtered channels and users</source>
<translation type="unfinished"></translation>
</message>
<message>
<source>Vie&amp;w Description</source>
<translation type="unfinished"></translation>
</message>
<message>
<source>View description in editor</source>
<translation type="unfinished"></translation>
</message>
<message>
<source>View description of channel %1</source>
<translation type="unfinished"></translation>
</message>
</context>
<context>
<name>Manual</name>
Expand Down
12 changes: 12 additions & 0 deletions src/mumble/mumble_eo.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7010,6 +7010,18 @@ Valid options are:
<source>Filtered channels and users</source>
<translation type="unfinished"></translation>
</message>
<message>
<source>Vie&amp;w Description</source>
<translation type="unfinished"></translation>
</message>
<message>
<source>View description in editor</source>
<translation type="unfinished"></translation>
</message>
<message>
<source>View description of channel %1</source>
<translation type="unfinished"></translation>
</message>
</context>
<context>
<name>Manual</name>
Expand Down
12 changes: 12 additions & 0 deletions src/mumble/mumble_es.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7153,6 +7153,18 @@ Las opciones válidas son:
<source>Filtered channels and users</source>
<translation>Canales filtrados y usuarias</translation>
</message>
<message>
<source>Vie&amp;w Description</source>
<translation type="unfinished"></translation>
</message>
<message>
<source>View description in editor</source>
<translation type="unfinished"></translation>
</message>
<message>
<source>View description of channel %1</source>
<translation type="unfinished"></translation>
</message>
</context>
<context>
<name>Manual</name>
Expand Down
12 changes: 12 additions & 0 deletions src/mumble/mumble_et.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6999,6 +6999,18 @@ Valid options are:
<source>Filtered channels and users</source>
<translation type="unfinished"></translation>
</message>
<message>
<source>Vie&amp;w Description</source>
<translation type="unfinished"></translation>
</message>
<message>
<source>View description in editor</source>
<translation type="unfinished"></translation>
</message>
<message>
<source>View description of channel %1</source>
<translation type="unfinished"></translation>
</message>
</context>
<context>
<name>Manual</name>
Expand Down
12 changes: 12 additions & 0 deletions src/mumble/mumble_eu.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7019,6 +7019,18 @@ Valid options are:
<source>Filtered channels and users</source>
<translation type="unfinished"></translation>
</message>
<message>
<source>Vie&amp;w Description</source>
<translation type="unfinished"></translation>
</message>
<message>
<source>View description in editor</source>
<translation type="unfinished"></translation>
</message>
<message>
<source>View description of channel %1</source>
<translation type="unfinished"></translation>
</message>
</context>
<context>
<name>Manual</name>
Expand Down
12 changes: 12 additions & 0 deletions src/mumble/mumble_fa_IR.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6999,6 +6999,18 @@ Valid options are:
<source>Filtered channels and users</source>
<translation type="unfinished"></translation>
</message>
<message>
<source>Vie&amp;w Description</source>
<translation type="unfinished"></translation>
</message>
<message>
<source>View description in editor</source>
<translation type="unfinished"></translation>
</message>
<message>
<source>View description of channel %1</source>
<translation type="unfinished"></translation>
</message>
</context>
<context>
<name>Manual</name>
Expand Down
Loading