Differential D1771

[Workspace Options] Add option to disable OSD
ClosedPublic
Actions

Authored by broulik on Jun 5 2016, 6:53 PM.

Details

Reviewers
graesslin
Group Reviewers
Plasma: Design
Plasma
Summary

As discussed with Usability.

CCBUG: 363983

Test Plan

Works. Also puts a bit more usefulness in that kcm.
It's not exclusively used for keyboard shortcuts but imho fair enough given that's the thing you could pack into a concise label.

Depends on D1770

Diff Detail

Repository
R119 Plasma Desktop
Lint
Lint Skipped
Unit
Unit Tests Skipped
broulik updated this revision to Diff 4219.Jun 5 2016, 6:53 PM
broulik retitled this revision from to [Workspace Options] Add option to disable OSD.
broulik updated this object.
broulik edited the test plan for this revision. (Show Details)
broulik added reviewers: Plasma, Plasma: Design.
broulik set the repository for this revision to R119 Plasma Desktop.
broulik added a dependency: D1770: [OSD] Allow disabling OSD through config file.
Restricted Application added a project: Plasma. · View Herald TranscriptJun 5 2016, 6:53 PM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
graesslin requested changes to this revision.Jun 6 2016, 5:47 AM
graesslin added a reviewer: graesslin.
graesslin added a subscriber: graesslin.
graesslin added inline comments.
kcms/workspaceoptions/mainpage.ui
60

The OSD will also be triggered for things like "disable touch pad when external mouse gets connected". So that wording seems wrong to me.

kcms/workspaceoptions/workspaceoptions.cpp
55

new connect syntax? (yes I am aware that changed is both a slot and a signal here and needs to be casted)

66

KSharedConfig::openConfig

75

for the readEntry you use QStringLiteral

90

QStringLiteral("OSD") is used twice in this file. Make it const static

90

as you only read from the cg you can make it const

This revision now requires changes to proceed.Jun 6 2016, 5:47 AM
broulik added a comment.Jun 6 2016, 8:30 AM
This comment was removed by broulik.
colomar added a subscriber: colomar.Jun 6 2016, 9:53 AM

Martin is right, OSDs are not only about keyboard shortcuts.

They are quite difficult to describe clearly without relying on technical terms, however.

Maybe "Visual feedback (On-Screen Display) for status changes not triggered via a graphical user interface"?
I think that's what they all have in common, because things done in the GUI should never trigger an OSD.

broulik added a comment.Sep 11 2016, 7:53 PM

Can we have something more concise than "Visual feedback (On-Screen Display) for status changes not triggered via a graphical user interface". Perhaps just "Visual feedback for status changes"?

colomar added a comment.Sep 11 2016, 9:13 PM
In D1771#50973, @broulik wrote:

Can we have something more concise than "Visual feedback (On-Screen Display) for status changes not triggered via a graphical user interface". Perhaps just "Visual feedback for status changes"?

Hm... or maybe give an example? "Visual feedback for status changes such as brightness or volume"?

andreaska added a subscriber: andreaska.Sep 11 2016, 9:18 PM

would it help if you add an "screenshot" where you show the information tip and osd?

broulik added a comment.Jan 9 2017, 12:43 PM

So what text should we go for then?

broulik added a comment.Jan 9 2017, 3:35 PM

I now used "Visual feedback for status changes" with a tooltip "Show a popup for status changes such as brightness or volume."

broulik updated this revision to Diff 9925.Jan 9 2017, 3:46 PM
broulik edited edge metadata.
  • Address issues
graesslin added inline comments.Jan 9 2017, 4:48 PM
kcms/workspaceoptions/workspaceoptions.cpp
56–58

why a lambda? It looks like you just could bind it to a signal

broulik updated this revision to Diff 9933.Jan 9 2017, 4:55 PM
broulik edited edge metadata.
  • cast function ptr instead of lambda
graesslin accepted this revision.Jan 9 2017, 8:16 PM
graesslin edited edge metadata.
This revision is now accepted and ready to land.Jan 9 2017, 8:16 PM
graesslin added a comment.Feb 19 2017, 4:38 PM

This one is pushed, isn't it?

broulik closed this revision.Feb 21 2017, 3:49 PM

Revision Contents
Changeset List

Diff 9933

View Options

kcms/workspaceoptions/mainpage.ui

Loading...
View Options

kcms/workspaceoptions/workspaceoptions.cpp

Loading...