Touchpad KCM Redesign Using Kirigami
ClosedPublic

Authored by furkantokac on May 27 2018, 2:18 AM.

Details

Summary

Touchpad KCM is redesigned by using Kirigami. Functionality is same. Tested in Wayland+Libinput and Xorg+Libinput. Works fine. Recommendations are welcome.

New design V0.5:

New design V0.4:

New design V0.3:

New design V0.2:

New design V0.1:

Old design:

Test Plan

Possible Missing Changes

  1. Acceleration profile should be enabled

Diff Detail

Repository
R119 Plasma Desktop
Branch
kcmtouchpad-kirigamidesign
Lint
No Linters Available
Unit
No Unit Test Coverage
furkantokac created this revision.May 27 2018, 2:18 AM
Restricted Application added a project: Plasma. · View Herald TranscriptMay 27 2018, 2:18 AM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
furkantokac requested review of this revision.May 27 2018, 2:18 AM
furkantokac edited the summary of this revision. (Show Details)May 27 2018, 2:23 AM
furkantokac added reviewers: ngraham, romangg, abetts.
furkantokac edited the summary of this revision. (Show Details)
furkantokac added inline comments.May 27 2018, 2:27 AM
kcms/touchpad/src/kcm/libinput/main.qml
467

This comment will be changed as "END Kirigami.ScrollablePage"

Comment correction.

ngraham added a comment.EditedMay 27 2018, 2:36 AM

In general much nicer!

Is the user-facing control for the click method (areas or clickfinger) going to come in a separate patch? It might make sense to add that feature first and then we can do all the UI review in a single patch.

Why is "acceleration profile" still disabled?

I also have some UI suggestions:

  • Let's try to avoid making this a scrollable page, if at all possible. If we can avoid that, we may be able to remove the scrollview.
  • "Emulate Middle button" -> We should come up with a less technical explanation for this. Suggestions welcome.
  • "Acceleration" -> "Speed" or "Pointer speed"
  • "Two tap right, three middle" -> "Tap twice for right-click; three times for middle-click"
  • "Two tap middle, three right" -> "Tap twice for middle-click; three times for right-click"
  • I don't think anyone will ever choose "No scroll" on purpose; this would just be a source of bugs. Let's remove that option.

Is the user-facing control for the click method (areas or clickfinger) going to come in a separate patch? It might make sense to add that feature first and then we can do all the UI review in a single patch.

Yes it'll be nice to having the feature with this patch. I'll implement it.

Why is "acceleration profile" still disabled?

It was like that for me all the time. @romangg is it okay to activate "acceleration profile" ?

I also have some UI suggestions:

  • Let's try to avoid making this a scrollable page, if at all possible. If we can avoid that, we may be able to remove the scrollview.

Currently the page is enough to keep everything. I'll convert it to page. Just want to know, why we should avoid using scrollable page ?

  • "Emulate Middle button" -> We should come up with a less technical explanation for this. Suggestions welcome.

At first, I didn't understand it too but tooltip helped. Tooltip is as following;
"Clicking left and right button simultaneously sends middle button click."
My suggestions are too long so it may not be appropriate. Lets think for sometime.

  • "Acceleration" -> "Speed" or "Pointer speed"

We can change "Acceleration" as "Speed" and "Acceleration Profile" as "Speed Profile". If you think that it is okay, I'll change it like that.

  • "Two tap right, three middle" -> "Tap twice for right-click; three times for middle-click"
  • "Two tap middle, three right" -> "Tap twice for middle-click; three times for right-click"

Yay!

  • I don't think anyone will ever choose "No scroll" on purpose; this would just be a source of bugs. Let's remove that option.

Actually yes. I'll remove it if there is no use case found by someone.

In general much nicer!

Very much thanks for your nice suggestions!

@romangg There are some "TODO"s in the code. Please tell me if they still need to be applied so I'll do them, if they are not, I'll remove them.

furkantokac added inline comments.May 27 2018, 3:08 AM
kcms/touchpad/src/kcm/libinput/main.qml
94

This will go if nobody tells me it is necessary. I couldn't find any use case that this will be important.

furkantokac edited the summary of this revision. (Show Details)May 27 2018, 3:09 AM

"Speed profile" sounds weird (in English, at least). For now, let's go with:

       Pointer speed: -------------I------
Acceleration profile: (o) Flat
                      ( ) Adaptive

As for wanting to making this non-scrollable: in general, we should aspire for the main views of our KCMs to never require scrolling (subviews like tables are okay, of course). It's very awkward to able to scroll a UI that's packed full of UI controls. It can even introduce issues since some controls respond to scroll events; it becomes easy to accidentally manipulate a control via a scroll event when you means to just move the view. Let's shoot for keeping all the content fully in view with a 1024x768 window size.

As for wanting to making this non-scrollable: in general, we should aspire for the main views of our KCMs to never require scrolling (subviews like tables are okay, of course). It's very awkward to able to scroll a UI that's packed full of UI controls. It can even introduce issues since some controls respond to scroll events; it becomes easy to accidentally manipulate a control via a scroll event when you means to just move the view. Let's shoot for keeping all the content fully in view with a 1024x768 window size.

I feel this is a "must". The vision is that we will empower the user to make changes quickly with minimal effort or digging. We already have a 3 layer deep system settings. Hopefully with this design, we can bring that number lower. Showing all settings within a window without scrolling is essential to that end. If you encounter a list that seems too long to fit in the space, bring it up to use and we can help.

  • "Two tap right, three middle" -> "Tap twice for right-click; three times for middle-click"
  • "Two tap middle, three right" -> "Tap twice for middle-click; three times for right-click"

May be clearer to use:
“Two tap right, three middle” → “Tap two fingers for right-click; three fingers for middle-click"
“Two tap middle, three right” → “Tap two fingers for middle-click; three fingers for right-click"

esedgh added a subscriber: esedgh.May 27 2018, 9:19 PM

Sorry guys I'm not sure if this is the appropriate place to report this as I'm not involved in the development.

But now that you're at it, I may give you a bit of feedback in here.

Touchpad settings has "Two Finger Scrolling" and "Edge Scrolling" as a radio button: You can choose only one.

But why? I'm used to having both enabled. Is that not doable in the new stack?

Sorry guys I'm not sure if this is the appropriate place to report this as I'm not involved in the development.

But now that you're at it, I may give you a bit of feedback in here.

Touchpad settings has "Two Finger Scrolling" and "Edge Scrolling" as a radio button: You can choose only one.

But why? I'm used to having both enabled. Is that not doable in the new stack?

All recommendations are welcome!

Very nice point. It should be as you said I think. I'll implement it if there is no opposite opinion.

Sadly, Libinput only allows one scrolling method to be active at once. See https://wayland.freedesktop.org/libinput/doc/latest/scrolling.html

So until and unless we can change Libinput itself, this has to stay as a radio button.

"NoScroll" setting is removed. Feedbacks are applied.

furkantokac edited the summary of this revision. (Show Details)May 28 2018, 11:47 PM
furkantokac edited the test plan for this revision. (Show Details)

Sadly, Libinput only allows one scrolling method to be active at once. See https://wayland.freedesktop.org/libinput/doc/latest/scrolling.html

So until and unless we can change Libinput itself, this has to stay as a radio button.

Thanks Nate! We can discuss the issue later then.

As for wanting to making this non-scrollable: in general, we should aspire for the main views of our KCMs to never require scrolling (subviews like tables are okay, of course). It's very awkward to able to scroll a UI that's packed full of UI controls. It can even introduce issues since some controls respond to scroll events; it becomes easy to accidentally manipulate a control via a scroll event when you means to just move the view. Let's shoot for keeping all the content fully in view with a 1024x768 window size.

I feel this is a "must". The vision is that we will empower the user to make changes quickly with minimal effort or digging. We already have a 3 layer deep system settings. Hopefully with this design, we can bring that number lower. Showing all settings within a window without scrolling is essential to that end. If you encounter a list that seems too long to fit in the space, bring it up to use and we can help.

Thanks. I got the issue. Is Page fully support the mobile phones or small-screen devices ? I supposed that we're using ScrollablePage to support more/different devices because I saw that some other KCMs using ScrollablePage too (Font, Translation, Launch KCM etc.) Is it okay to use Page for small screen devices ?

The middle click emulation issue is tricky because the behavior and available options are different depending on the hardware configuration, and, if using a buttonless touchpad, also the click method used. I wonder if we need to make the UI dynamically adapt to the aforementioned possibilities, like this:

1. Touchpad with hardware buttons

Show a checkbox like this:

Middle click: [X] By pressing left and right buttons simultaneously

2. Touchpad with no hardware buttons when "clickfinger" click method is used

Change the checkbox to have the following text:

Middle click: [X] By pressing on the touchpad with three fingers

3. Touchpad with no hardware buttons when "areas" click method is used

Instead show radio buttons like this:

Middle click: (o) By pressing virtual left and right buttons simultaneously
              ( ) With a virtual button between virtual left and right buttons
ngraham added inline comments.May 29 2018, 12:03 AM
kcms/touchpad/src/kcm/libinput/main.qml
373

"Two finger tap to right-click; three finger tap to middle-click"

374

"Two finger tap to middle-click; three finger tap to right-click"

Another idea: omit the always-disabled "Device" combobox when there's only a single device. It's not doing much good in that situation.

ngraham added inline comments.May 29 2018, 12:06 AM
kcms/touchpad/src/kcm/libinput/main.qml
439

Idea: change this string to:

Invert scroll direction ("Natural scrolling")

This might help improve discoverability for people who are familiar with the term "Natural scrolling", which is used by Apple and GNOME.

ngraham added a comment.EditedMay 29 2018, 3:28 AM

Here's another idea for the multi-tapping radio button to reduce the string length:

Two-finger tap: (o) Right-click (three-finger tap to middle-click)
                ( ) Middle-click (three-finger tap to right-click)

Here's another idea for the multi-tapping radio button to reduce the string length:

Two-finger tap: (o) Right-click (three-finger tap to middle-click)
                ( ) Middle-click (three-finger tap to right-click)

Elegant! I was looking for something like this. Done :)

furkantokac updated this revision to Diff 35527.EditedJun 4 2018, 1:50 PM

Tapping strings are changed. Natural scrolling string is added. If there is only 1 touchpad, device combobox set to insvisible. ScrollablePage is changed with Page.

furkantokac edited the summary of this revision. (Show Details)Jun 4 2018, 1:52 PM
furkantokac edited the test plan for this revision. (Show Details)
furkantokac edited the summary of this revision. (Show Details)Jun 4 2018, 2:17 PM
romangg requested changes to this revision.Jun 4 2018, 2:22 PM
romangg added inline comments.
kcms/touchpad/src/kcm/libinput/main.qml
104

I'm against this change for several reasons:

  • In general control elements shouldn't be completely hidden but only disabled to improve discoverability of features.
  • When a user has the KCM open and he connects a second touchpad or removes the first one the whole Ui shifts down/up.
  • While being greyed out the combobox still shows the current device name in case there is only one connected and therefore gives valuable information to the user.
This revision now requires changes to proceed.Jun 4 2018, 2:22 PM
furkantokac updated this revision to Diff 35545.EditedJun 4 2018, 6:06 PM

V0.4. Combobox is always visible. Combobox manual resizing is removed.

furkantokac marked 5 inline comments as done.Jun 4 2018, 6:07 PM
furkantokac edited the summary of this revision. (Show Details)Jun 4 2018, 6:09 PM

If we're keeping the combobox on top, it needs to be wide enough to show the full string. Otherwise it's not only functionally useless 99.99% of the time, it's also uninformative.

ngraham requested changes to this revision.Jun 4 2018, 6:18 PM
ngraham added inline comments.
kcms/touchpad/src/kcm/libinput/main.qml
194

"Press left and right buttons for middle click"

This revision now requires changes to proceed.Jun 4 2018, 6:18 PM
furkantokac updated this revision to Diff 35554.EditedJun 4 2018, 7:10 PM

V0.5. Combobox is expanded. MiddleEmulation string is changed.

furkantokac edited the summary of this revision. (Show Details)Jun 4 2018, 7:31 PM
ngraham accepted this revision.Jun 4 2018, 8:45 PM

Looks grand now!

This revision was not accepted when it landed; it landed in state Needs Review.Jun 4 2018, 9:05 PM
This revision was automatically updated to reflect the committed changes.
furkantokac edited the summary of this revision. (Show Details)Jun 5 2018, 12:04 AM