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

Add per-notification setting to toggle window highlight (if not active) #3354

Merged
merged 2 commits into from Mar 2, 2018

Conversation

@davidebeatrici
Copy link
Member

commented Mar 1, 2018

As requested in #3283.

@bontibon

This comment has been minimized.

Copy link
Contributor

commented Mar 1, 2018

Shouldn't this be configurable? I can see how some users would not like this feature.

@davidebeatrici

This comment has been minimized.

Copy link
Member Author

commented Mar 1, 2018

As a global setting or for each notification?

@davidebeatrici davidebeatrici requested review from mkrautz, Kissaki, bontibon and hacst Mar 1, 2018

@mkrautz

This comment has been minimized.

Copy link
Member

commented Mar 1, 2018

Global setting.

@davidebeatrici

This comment has been minimized.

Copy link
Member Author

commented Mar 1, 2018

Should it be in the User Interface or Messages tab?

@mkrautz

This comment has been minimized.

Copy link
Member

commented Mar 1, 2018

Actually, it'd probably make most sense as a per-notification setting. People probably want to get Mumble highlighted for some notifications, not others. Text messages would be a good example.

How about calling it "Highlight"?

@davidebeatrici

This comment has been minimized.

Copy link
Member Author

commented Mar 1, 2018

I agree, having a per-notification setting would be perfect.

"Highlight" sounds good, with a tooltip explaining what it means in detail.

@davidebeatrici davidebeatrici force-pushed the davidebeatrici:window-alert branch from d754305 to 3fd00f4 Mar 1, 2018

@davidebeatrici

This comment has been minimized.

Copy link
Member Author

commented Mar 1, 2018

@davidebeatrici davidebeatrici changed the title Log: highlight window in taskbar if it's not active Add per-notification setting to toggle window highlight in taskbar Mar 1, 2018

twi->setCheckState(ColStaticSound, Qt::Unchecked);

twi->setToolTip(ColConsole, tr("Toggle console for %1 events").arg(messageName));
twi->setToolTip(ColNotification, tr("Toggle pop-up notifications for %1 events").arg(messageName));
twi->setToolTip(ColHighlight, tr("Toggle window highlight in taskbar (if not active) for %1 events").arg(messageName));

This comment has been minimized.

Copy link
@mkrautz

mkrautz Mar 2, 2018

Member

Taskbar is not a cross-platform term.

This comment has been minimized.

Copy link
@davidebeatrici

davidebeatrici Mar 2, 2018

Author Member

What about "Application bar"?

This comment has been minimized.

Copy link
@mkrautz

mkrautz Mar 2, 2018

Member

How about just dropping "in taskbar"? Same for the rest of these.

This comment has been minimized.

Copy link
@davidebeatrici

davidebeatrici Mar 2, 2018

Author Member

Sounds good.

Every desktop environment has its own way to highlight the window (if supported).

twi->setToolTip(ColStaticSound, tr("Click here to toggle sound notification for %1 events").arg(messageName));
twi->setToolTip(ColStaticSoundPath, tr("Path to sound file used for sound notifications in the case of %1 events<br />Single click to play<br />Double-click to change").arg(messageName));

twi->setWhatsThis(ColConsole, tr("Click here to toggle console output for %1 events.<br />If checked, this option makes Mumble output all %1 events in its message log.").arg(messageName));
twi->setWhatsThis(ColNotification, tr("Click here to toggle pop-up notifications for %1 events.<br />If checked, a notification pop-up will be created by Mumble for every %1 event.").arg(messageName));
twi->setWhatsThis(ColHighlight, tr("Click here to toggle window highlight in taskbar for %1 events.<br />If checked, Mumble's window will be highlighted in the taskbar for every %1 event, if not active.").arg(messageName));

This comment has been minimized.

Copy link
@mkrautz

mkrautz Mar 2, 2018

Member

Taskbar is not a cross-platform term.

if ((flags & Settings::LogBalloon) && !(g.mw->isActiveWindow() && g.mw->qdwLog->isVisible()))
postNotification(mt, plain);
if (!(g.mw->isActiveWindow() && g.mw->qdwLog->isVisible())) {
// Message notification with window highlight in taskbar

This comment has been minimized.

Copy link
@mkrautz

mkrautz Mar 2, 2018

Member

Taskbar is not a cross-platform term.

@davidebeatrici davidebeatrici force-pushed the davidebeatrici:window-alert branch from 3fd00f4 to 21eb9cc Mar 2, 2018

@davidebeatrici davidebeatrici changed the title Add per-notification setting to toggle window highlight in taskbar Add per-notification setting to toggle window highlight (if not active) Mar 2, 2018

Translation update
Updating 'mumble_en.ts'...
    Found 1700 source text(s) (3 new and 1697 already existing)
@mkrautz
mkrautz approved these changes Mar 2, 2018
Copy link
Member

left a comment

LGTM. Wait for builds to be green.

@davidebeatrici davidebeatrici merged commit ce8fd36 into mumble-voip:master Mar 2, 2018

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.