D25375: Start of the accessibility KCM

Carson Black noreply at phabricator.kde.org
Tue Jan 7 03:13:11 GMT 2020


cblack added a comment.


  The QML code could use some help in the style department. Some changes I would do:
  
  1. Make ID separate from other properties
  2. Chunk together related properties in a consistent manner
  
  https://doc.qt.io/qt-5/qml-codingconventions.html and https://community.kde.org/Plasma/QMLStyle are relevant here.
  
  Also, I wonder if it would be better to spin out each tab into a KCM of its own instead of retaining a single Accessibility KCM as a catch-all for accessibility stuff that doesn't go anywhere else.

INLINE COMMENTS

> CMakeLists.txt:17
>  
> -target_link_libraries(kcm_access
> -    Qt5::X11Extras
> +message("Arquivos" ${kcmaccess_PART_SRCS})
> +target_link_libraries(kcmaccess

What purpose does this serve? It looks like you're simply printing a list of source files with a Portuguese header.

> kcmaccess.cpp:194
> +
>      int ret = QProcess::execute(QStringLiteral("gsettings"), gsettingArgs);
>      if (ret) {

You could consider using glib for a stable ABI instead of invoking a command line that could change at any time. Also would reduce dependencies for most distros, as glib's command line tools are often packaged separately from glib itself.

> ModifierKeys.qml:98
> +        icon.name: "preferences-desktop-notification"
> +        onClicked: kcm.keyboardSettings.configureKNotify();
> +    }

Redundant semicolon; there's only one statement on this line.

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

There should be an ellipsis here since this is opening another configuration window.

REPOSITORY
  R119 Plasma Desktop

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

To: tcanabrava, ngraham, ervin
Cc: cblack, ervin, ognarb, mart, ngraham, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, zachus, 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/20200107/edc4a7e9/attachment-0001.html>


More information about the Plasma-devel mailing list