Differential D19690

Improve SNI text
ClosedPublic
Actions

Authored by apol on Mar 11 2019, 4:54 PM.

Details

Reviewers
ngraham
Group Reviewers
Plasma
KWin
Commits
R108:c192d3519281: Improve SNI text
Summary

Allow the user to see if rotation lock is enabled by reading the text.
Alternatively one has to hover it and wait for the tooltip.

Diff Detail

Repository
R108 KWin
Branch
orientationText
Lint
Lint SkippedExcuse: Can't install cppcheck for reasons
Unit
No Unit Test Coverage
Build Status
Buildable 9481
Build 9499: arc lint + arc unit
apol created this revision.Mar 11 2019, 4:54 PM
Restricted Application added a project: KWin. · View Herald TranscriptMar 11 2019, 4:54 PM
Restricted Application added a subscriber: kwin. · View Herald Transcript
apol requested review of this revision.Mar 11 2019, 4:54 PM
zzag added a subscriber: zzag.Mar 11 2019, 6:59 PM
zzag added inline comments.
orientation_sensor.cpp
120

Can't we address this todo item instead?

ndavis added a subscriber: ndavis.EditedMar 13 2019, 7:38 AM

Would it make sense to have 3 icons for auto-rotate, locked to portrait and locked to landscape?

apol added a comment.Mar 13 2019, 2:33 PM
In D19690#430032, @ndavis wrote:

Would it make sense to have 3 icons for auto-rotate, locked to portrait and locked to landscape?

I don't think so, if it's locked, you should already see it.

ndavis added a comment.Mar 13 2019, 4:27 PM

Where is this icon seen? What sizes are needed?

bruns added a subscriber: bruns.Mar 13 2019, 7:39 PM
arvidhansson added a comment.Mar 17 2019, 1:27 PM

What type of item is it(https://hig.kde.org/style/icon.html)? Is there any way to download the current image?

ndavis added a comment.EditedMar 17 2019, 10:55 PM
In D19690#432671, @arvidhansson wrote:

What type of item is it(https://hig.kde.org/style/icon.html)? Is there any way to download the current image?

The current image is preferences-desktop-display from the breeze-icons repo.

I'm making a replacement here: D19736

apol added a comment.Mar 19 2019, 3:50 PM
orientation_sensor.cpp
120

I don't think that's sufficient. I'll improve the texts.

apol updated this revision to Diff 54334.Mar 19 2019, 3:57 PM

Iterate texts

ngraham added a subscriber: ngraham.Mar 19 2019, 4:03 PM
ngraham added inline comments.
orientation_sensor.cpp
72

"Allow Automatic Rotation" ?

81

Can we just not show any text in this state? "Undefined" is not really a useful user-facing string.

85

How about: "Locked: Portrait View", and the other is "Locked: Landscape View"

?

apol added inline comments.Mar 19 2019, 4:20 PM
orientation_sensor.cpp
72

Rotating automatically?
On Android it says "Auto-rotate".

81

I wouldn't worry too much, this should never appear.

85

I'm not convinced about the "View" bit, maybe Display would be a bit better?

apol added inline comments.Mar 19 2019, 4:43 PM
orientation_sensor.cpp
85

I also wonder if we need to say Locked at all.

apol added inline comments.Mar 19 2019, 8:09 PM
orientation_sensor.cpp
72

"Allow Rotation"

apol updated this revision to Diff 54363.Mar 19 2019, 8:52 PM

Iterate text as discussed with nate

ngraham accepted this revision.Mar 19 2019, 8:58 PM

Shipit!

This revision is now accepted and ready to land.Mar 19 2019, 8:58 PM
Closed by commit R108:c192d3519281: Improve SNI text (authored by apol). · Explain WhyMar 19 2019, 9:04 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Morientation_sensor.h (1 line)
Morientation_sensor.cpp (5 lines)

Diff 53679

View Options

orientation_sensor.h

Loading...
View Options

orientation_sensor.cpp

Loading...