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

Allow only one instance from UserLocalVolumeDialog per User #2284

Closed
wants to merge 4 commits into from

Conversation

@SuperNascher
Copy link
Contributor

commented May 18, 2016

An extension for #1903 (Improve Volume adjust per user) because it is possible to open more than one volume dialog per user.

If the user tries now to open a dialog for a user that is already open, Mumble will show the dialog instead of open a new one.

@mkrautz mkrautz self-assigned this May 18, 2016


public:
UserLocalVolumeDialog(unsigned int sessionId = 0);
UserLocalVolumeDialog(unsigned int sessionId = 0,
QMap<unsigned int, UserLocalVolumeDialog*> *qmuservolTracker = NULL);

This comment has been minimized.

Copy link
@mkrautz

mkrautz May 18, 2016

Member

The class can't work with qmuservolTracker == NULL, so don't use a default value here.

@@ -71,6 +72,7 @@ class MainWindow : public QMainWindow, public MessageHandler, public Ui::MainWin
QMenu *qmTray;
QIcon qiIcon, qiIconMuteSelf, qiIconMuteServer, qiIconDeafSelf, qiIconDeafServer, qiIconMuteSuppressed;
QIcon qiTalkingOn, qiTalkingWhisper, qiTalkingShout, qiTalkingOff;
QMap<unsigned int, UserLocalVolumeDialog*> qmuservolTracker;

This comment has been minimized.

Copy link
@mkrautz

mkrautz May 18, 2016

Member

s/qmuservolTracker/qmUserVolTracker/ (and all future occurances)

Use a space between UserLocalVolumeDialog and *.

@@ -41,9 +41,11 @@
#include "ClientUser.h"
#include "Database.h"

UserLocalVolumeDialog::UserLocalVolumeDialog(unsigned int sessionId)
UserLocalVolumeDialog::UserLocalVolumeDialog(unsigned int sessionId,
QMap<unsigned int, UserLocalVolumeDialog*> *qmuservolTracker)

This comment has been minimized.

Copy link
@mkrautz

mkrautz May 18, 2016

Member

Use a space between UserLocalVolumeDialog and *.

@mkrautz mkrautz removed their assignment May 18, 2016

@mkrautz

This comment has been minimized.

Copy link
Member

commented May 18, 2016

Not too happy about the logic being spread across MainWindow and UserLocalVolumeDialog.

Instead of the code in MainWindow, how about a static method (show?) on UserLocalVolumeDialog that takes the same parameters as the constructor, but contains the code that's currently in void MainWindow::on_qaUserLocalVolume_triggered()?

So:

void UserLocalVolumeDialog::show(unsigned int sessionId, QMap<unsigned int, UserLocalVolumeDialog *> *qmUserVolTracker) {
    if (qmUserVolTracker->contains(session)) {
        qmUserVolTracker->value(session)->raise();
    } else {
        UserLocalVolumeDialog *uservol = new UserLocalVolumeDialog(sessionId, qmUserVolTracker);
        uservol->show();
        qmUserVolTracker->insert(session, uservol);
    }
}

?

@mkrautz mkrautz self-assigned this May 19, 2016

@mkrautz

This comment has been minimized.

Copy link
Member

commented May 19, 2016

Landed via a779870.

@mkrautz mkrautz closed this May 19, 2016

@SuperNascher SuperNascher deleted the SuperNascher:uservolume branch May 19, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.