Layout and animations on the on screen keyboard
ClosedPublic

Authored by mart on Mar 2 2017, 2:28 PM.

Details

Summary

use a state machine to define in a cleaner way
the layout of the lock screen when the keyboard is
open on closed. animate between the state changes with
a slide animation
Depends on D4870

Test Plan

tested opening the keyboard with different screen locker sizes
to see the text field and the button always stay visible

Diff Detail

Repository
R120 Plasma Workspace
Branch
Branch2_backup
Lint
No Linters Available
Unit
No Unit Test Coverage
mart created this revision.Mar 2 2017, 2:28 PM
Restricted Application added a project: Plasma. · View Herald TranscriptMar 2 2017, 2:28 PM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
graesslin requested changes to this revision.Mar 15 2017, 6:28 PM

Honestly I don't think it's a good idea to animate the y position of the main stack. I fear this can result in a completely unusable lock screen as the main elements might get outside the visible area. I had huge problems with the layout of the lockscreen and fiddled quite a bit till I had it working correctly. Compare the problems with the Clock and scaling. I went for overlapping with parts of the UI for a reason as I don't think we have enough screen estate to show both the full UI and the keyboard.

lookandfeel/contents/lockscreen/LockScreenUi.qml
71

Why the change to MouseArea?

81

This is not related to the described change. Do we really want to close when clicking outside the keyboard? That could be rather annoying? Anyway I think it should not be bundled with a change saying it's about animations

112

Could you please rebase this to the changes introduces with 097db85e297aba8b4b3f0ddabcddf8a03b5482c0

228

I'm especially afraid of reintroducing units here with a fixed multiplier. That was exactly the thing causing problems with scaling and the clock.

This revision now requires changes to proceed.Mar 15 2017, 6:28 PM
mart updated this revision to Diff 12513.Mar 16 2017, 11:49 AM
mart edited edge metadata.
  • adress comments
lookandfeel/contents/lockscreen/LockScreenUi.qml
71

to dismiss the keyboard by clicking anywhere in an empty area

81

that's how on screen keyboards usually work on pretty much any device.
removed from this, will do a separate review request about it

112

i see that commit puts the clock in a column layout together with just an empty item.. why? looks like some easy cleanup can be done there

in adapting to that diff, now hiding the user icon when it goes over the clock, but i think the clock sliding away was actually more correct ui-wise

228

this is not really a functional position:
this is when the keyboard is hidden and its opacity is 0.
it's to not make it slide all the way, to emulate the slidingpopups effect, as it should have a similar behavior.
changed to a relative one, so the kwyboard always slides for a quarter for its length while appearing/disappearing, that's should be more different resolutions-proof

mart added inline comments.Mar 16 2017, 11:53 AM
lookandfeel/contents/lockscreen/LockScreenUi.qml
112

purely ui-wise, i think the correct behavior of the clock (for a future review) should be:

  • stay at top of the screen
  • either disappear or slide away when the user icon starts to overlap it. Either always disappears, or slides when it's pushed away when the keyboard is open and disappears when it's pushed away and the keyboard is closed (very very tiny screen resolution)
  • user icon disappears as before when it starts to go out of the screen
graesslin added inline comments.Mar 16 2017, 4:38 PM
lookandfeel/contents/lockscreen/LockScreenUi.qml
81

that's how on screen keyboards usually work on pretty much any device.

Fair enough, but then it should be a Touch enabled field and not a MouseArea?

plasma.desktop.cmake
55

erm?

mart added inline comments.Mar 16 2017, 5:02 PM
lookandfeel/contents/lockscreen/LockScreenUi.qml
81

good point

plasma.desktop.cmake
55

ouch, is a sideeffect of the rebasing i guess :/

graesslin added inline comments.Mar 16 2017, 5:40 PM
applets/systemtray/systemtray.cpp
201

That looks like an unrelated change?

lookandfeel/contents/lockscreen/LockScreenUi.qml
112

yeah hiding the clock if it overlaps with the user icon would be a good thing. I didn't see an easy way for it, but well me and QML ;-)

291

note that the previous code did not set the active to false. The idea was that once you activated the virtual keyboard, clicking/touching into the password field also activates if. For that the inputPanel needs to be active.

In summary: if you thought I just had forgotten to set it to false, nope, it was intended ;-)

mart updated this revision to Diff 12529.Mar 16 2017, 6:07 PM
mart marked an inline comment as done.

new clock layouting strategy

mart updated this revision to Diff 12530.Mar 16 2017, 6:08 PM

remove unintended change

mart added inline comments.Mar 16 2017, 6:10 PM
lookandfeel/contents/lockscreen/LockScreenUi.qml
112

this way seems to behave as intended, i tried to resize the lockscreen test window to various sizes that may be challenging and seems to behave quite well

mart updated this revision to Diff 13138.Apr 6 2017, 9:47 AM
  • use activate
mart updated this revision to Diff 13140.Apr 6 2017, 9:58 AM
  • use activate
  • remove change made by mistake
  • fix merge errors
mart updated this revision to Diff 13141.Apr 6 2017, 10:01 AM
  • modeltest again as in master
mart updated this revision to Diff 13142.Apr 6 2017, 10:01 AM
  • unsigned
mart updated this revision to Diff 13143.Apr 6 2017, 10:02 AM
  • fix build
broulik accepted this revision.Apr 6 2017, 10:04 AM
This revision was automatically updated to reflect the committed changes.

I fear this change caused regressions. I now don't have the virtual keyboard button on the layout anymore (X11 and Wayland) and on Wayland it's impossible to unlock the screen.

I fear this change caused regressions. I now don't have the virtual keyboard button on the layout anymore (X11 and Wayland) and on Wayland it's impossible to unlock the screen.

I just reverted the change locally and that fixed the issue for me. So yes this change unfortunately broke the feature it was supposed to improve. As an idea: maybe it did not incorporate adjustements for 54479d32a96430dd093b4489c6f2a6fe449d2f80.

mart added a comment.Apr 20 2017, 5:50 PM

I fear this change caused regressions. I now don't have the virtual keyboard button on the layout anymore (X11 and Wayland) and on Wayland it's impossible to unlock the screen.

I just reverted the change locally and that fixed the issue for me. So yes this change unfortunately broke the feature it was supposed to improve. As an idea: maybe it did not incorporate adjustements for 54479d32a96430dd093b4489c6f2a6fe449d2f80.

so is the virtual keyboard button itself that's invisible? that shouldn't be touched by this patch.
In my master https://phabricator.kde.org/R120:54479d32a96430dd093b4489c6f2a6fe449d2f80 is in (and the button is visible)

In D4893#103687, @mart wrote:

I fear this change caused regressions. I now don't have the virtual keyboard button on the layout anymore (X11 and Wayland) and on Wayland it's impossible to unlock the screen.

I just reverted the change locally and that fixed the issue for me. So yes this change unfortunately broke the feature it was supposed to improve. As an idea: maybe it did not incorporate adjustements for 54479d32a96430dd093b4489c6f2a6fe449d2f80.

so is the virtual keyboard button itself that's invisible?

Yes that's one of the problems. The other is that on Wayland the button also doesn't show and nevertheless the keyboard loads directly, but text entering is not possible through virt keyboard nor through physical keyboard.

Just verified: reverting this change also fixes the issues mentioned for Wayland.

mart added a comment.Apr 21 2017, 9:40 AM

Just verified: reverting this change also fixes the issues mentioned for Wayland.

i think i found the problem, it was looking for VirtualKeyboard.qml in the wrong path, now it should work

In D4893#103772, @mart wrote:

Just verified: reverting this change also fixes the issues mentioned for Wayland.

i think i found the problem, it was looking for VirtualKeyboard.qml in the wrong path, now it should work

Thanks for investigating! I can confirm that your commit fixed the issue and as it nicely animates I'm running your latest version ;-)

The wrong path happened through 78d2c9ffbed64c12578e8b94aead3ecad6bf48bb which moved the file to components. And now I also understand the issue on Wayland. Due to the file not being there, kscreenlocker_greet created a standalone virtual keyboard (which was the one I saw) and broke my keyboard input.