D25375: Start of the accessibility KCM

Nathaniel Graham noreply at phabricator.kde.org
Mon Dec 2 17:07:38 GMT 2019


ngraham requested changes to this revision.
ngraham added a comment.
This revision now requires changes to proceed.


  All the left labels in the formLayouts need to end with a colon, e.g.:
  
    Delay: [ 0]
  
  Don't use `onCheckStateChanged` or `onCheckedChanged` for checkboxes when you want to handle user interaction;i t causes binding loops. Use `onToggled` instead.
  
  Put whitespace between logical sections in the formlayout. For example between the audible bell and visual bell sections. You can do this like so:
  
    Item {
        Kirigami.FormData.isSection: true
    }
  
  Change `QtQuick.Controls 2.12` to `QtQuick.Controls 2.11` everywhere; we don't formally depends on Qt 5.12 yet.

INLINE COMMENTS

> ActivationGestures.qml:29
> +    QQC2.CheckBox {
> +        Kirigami.FormData.label: i18n("Activation Gestures")
> +        text: i18n("Use gestures for activating sticky keys and slow keys")

This section can be expressed more clearly and compactly as follows:

  Sticky and slow keys: [ ] Use gestures to activate
                        [ ] Disable after: [99 seconds ^v]

In fact, this section maybe should be moved to the Modifier Keys page, and then we can remove the Activation Gestures page entirely.

> ActivationGestures.qml:30
> +        Kirigami.FormData.label: i18n("Activation Gestures")
> +        text: i18n("Use gestures for activating sticky keys and slow keys")
> +        checked: kcm.keyboardSettings.stickyKeys

Can be shortened to "Use gestures for activating sticky and slow keys"

> ActivationGestures.qml:50
> +    QQC2.CheckBox {
> +        Kirigami.FormData.label: i18n("Notification")
> +        text: i18n("Use the system bell whenever a gesture is used to toggle an accessibility feature")

This section doesn't have anything to do with activation gestures. Its checkboxes might be better placed on other pages more relevant to their content.

> ActivationGestures.qml:69
> +    QQC2.Button {
> +        text: i18n("Configure Notifications")
> +        onClicked: kcm.configureKNotify();

add ellipsis and an icon (`preferences-desktop-notification`)

> ModifierKeys.qml:38
> +        enabled: !kcm.keyboardSettings.isImmutable("StickyKeysLatch") && kcm.keyboardSettings.stickyKeys
> +        text: i18n("Lock sticky keys")
> +        checked: kcm.keyboardSettings.stickyKeysLatch

Can be shortened to just "Lock"

> ModifierKeys.qml:45
> +        enabled: !kcm.keyboardSettings.isImmutable("StickyKeysAutoOff") && kcm.keyboardSettings.stickyKeys
> +        text: i18n("Turn off sticky keys when two keys are pressed continuously")
> +        checked: kcm.keyboardSettings.stickyKeysAutoOff

Can be shortened to "Disable when two keys are held down"

> ModifierKeys.qml:51
> +        enabled: !kcm.keyboardSettings.isImmutable("StickyKeysBeep") && kcm.keyboardSettings.stickyKeys
> +        text: i18n("Use system bell whenever a modifier gets latched, locked or unlocked")
> +        checked: kcm.keyboardSettings.stickyKeysBeep

Can be shortened to "Use system bell when modifier keys are used" (I *think* this is what the string is trying to  communicate; correct me if I'm wrong)

> ModifierKeys.qml:58
> +        Kirigami.FormData.label: i18n("Locking Keys")
> +        text: i18n("Use system bell whenever a locking key gets toggled")
> +        checked: kcm.keyboardSettings.toggleKeysBeep

Can be slightly shortened to "Use system bell when locking keys are toggled"

> ModifierKeys.qml:64
> +    QQC2.CheckBox {
> +        text: i18n("Show a notification for modifier or locking key state changes")
> +        checked: kcm.keyboardSettings.kNotifyModifiers

Can be shortened to "Show notification when modifier or locking keys are used"

> MouseNavigation.qml:31
> +        Kirigami.FormData.label:  i18n("Activation Gestures")
> +        text: i18n("Move pointer with keyboard (using the num pad)")
> +        checked: kcm.mouseSettings.mouseKeys

Change to "Enable using keyboard number pad to move cursor"

> MouseNavigation.qml:35
> +        enabled: !kcm.mouseSettings.isImmutable("MouseKeys")
> +    }
> +    QQC2.SpinBox {

Add spacing after this

> ScreenReader.qml:30
> +        Kirigami.FormData.label:  i18n("Screen Reader")
> +        text: i18n("enabled")
> +        checked: kcm.screenReaderSettings.enabled

In this case, just put the whole text in the checkbox:

  [ ] Enable screen reader
  [ Launch Orca Configuration...]

> ScreenReader.qml:36
> +    QQC2.Button {
> +        text: i18n("Launch Orca Configuration")
> +        onClicked: kcm.screenReaderSettings.launchOrcaConfiguration()

What's Orca? Isn't that a kind of whale? :p Make change the text to be "Configure Orca Screen Reader..."

> ScreenReader.qml:37
> +        text: i18n("Launch Orca Configuration")
> +        onClicked: kcm.screenReaderSettings.launchOrcaConfiguration()
> +        enabled: !kcm.screenReaderSettings.isImmutable("Enabled")

If Orca isn't installed, clicking on this button does nothing. In this case, it should be disabled with a message below it explaining why it's disabled.

> main.qml:37
> +        QQC2.ScrollView {
> +            id: bgObject
> +            Component.onCompleted: bgObject.background.visible = true

should be more descriptive, like `sidebarScrollView` or something

> main.qml:50
> +
> +                delegate: Kirigami.BasicListItem {
> +                    width: listView.width

needs to force focus to the scrollview when clicked: `onClicked: listView.forceActiveFocus();`

> main.qml:53
> +                    icon: model.icon
> +                    label: i18n(model.title)
> +                }

title should already be translated; no need to put this in an `i18n()` call

> main.qml:77
> +            icon: "preferences-desktop-notification-bell"
> +            title: QT_TR_NOOP("Bell")
> +        }

Shouldn't these be `i18n()` calls?

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D25375

To: tcanabrava, ngraham
Cc: ognarb, mart, ngraham, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, GB_2, ragreen, ZrenBot, alexeymin, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20191202/43950ad6/attachment-0001.html>


More information about the Plasma-devel mailing list