sebas added inline comments. INLINE COMMENTS
> kwin_wayland.cmake:1 > +#find_package(X11 REQUIRED) > + remove commented out bits? > kwinwaylandbackend.cpp:46 > + m_deviceManager->connection().connect(QStringLiteral("org.kde.KWin"), > + > QStringLiteral("/org/kde/KWin/InputDevice"), > + > QStringLiteral("org.kde.KWin.InputDeviceManager"), indenting four more spaces aligns it with the above Yes, OCD. :) > kwinwaylandbackend.cpp:89 > + KWinWaylandTouchpad* tp = new KWinWaylandTouchpad(sn); > + if (! tp->init()) { > + qCCritical(KCM_TOUCHPAD) << "Error on creating touchpad > object" << sn; remove whitespace between ! and tp > kwinwaylandbackend.h:2 > +/* > + * Copyright (C) 2016 Roman Gilg <subd...@gmail.com> > + * Bit of a nitpick, but the (C) doesn't really add anything. I usually leave it out. > kwinwaylandtouchpad.h:36 > + Q_PROPERTY(QString name READ name CONSTANT) > + Q_PROPERTY(bool supportsDisableEvents READ supportsDisableEvents > CONSTANT) > + Q_PROPERTY(bool enabled READ isEnabled WRITE setEnabled NOTIFY > enabledChanged) I wonder if it wouldn't be neater if the features would be an enum, and you'd have methods to check for supported and enabled features. Have you thought of this? > kwinwaylandtouchpad.h:352 > + struct Prop { > + explicit Prop(QByteArray dbusName) > + : dbus(dbusName) const & for the argument? > ToolTip.qml:24 > +MouseArea { > + anchors.fill: parent > + whitespace looks weird here ... perhaps group the properties, the new properties and the signal handlers in blocks? > ToolTip.qml:45 > + > + function killTooltip() { > + stop() Could also be hidden by using a onRunningChanged handler, that way the stop() method would 'just work' > main.qml:274 > + } > + value = 4.5 * > touchpad.pointerAcceleration + 5.5 > + } magic values, a comment would be useful where the 4.5 and 5.5 come from > main.qml:279 > + // return on initializing, > otherwise ugly type error message by qml > + if (!initialized) { > + initialized = true is "value changed twice" a useful condition to set initalized here? > main.qml:285 > + if (enabled && > !root.loading) { > + > touchpad.pointerAcceleration = Math.round( (value - 5.5) / 4.5 * 100 ) / 100 > + root.changeSignal() same here, comment please > main.qml:294 > + hoverEnabled: true > + onPressed: mouse.accepted = > false > + onWheel: { /* ignore. TODO: > send event to ScrollView */ } not so sure about that, it would introduce inconsistent special behaviour just in this KCM, as a slider always should react to wheel events. > main.qml:545 > + ToolTip { > + text: i18n("Touchscreen like > scrolling") > + } full stop is missing at the end > touchpadconfiglibinput.cpp:100 > +{ > + return QSize(650,800); > +} This will fall over with high dpi displays... > touchpadconfiglibinput.cpp:105 > +{ > + return QSize(100,200); > +} problematic for high dpi REPOSITORY R119 Plasma Desktop REVISION DETAIL https://phabricator.kde.org/D3617 EMAIL PREFERENCES https://phabricator.kde.org/settings/panel/emailpreferences/ To: subdiff, #kwin, #plasma_on_wayland, #plasma, #vdg Cc: sebas, luebking, graesslin, knambiar, kwin, plasma-devel, lesliezhai, ali-mohamed, hardening, jensreuterberg, abetts, eliasp