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