liquidshell in kdereview

Kai Uwe Broulik kde at privat.broulik.de
Sat Nov 4 18:41:27 GMT 2017


Hi,

AppMenu.cxx:
* You can use NoDotAndDotDot flag in entryInfoList() to already skip those
* You seem to be using KFileItem and the url you generate just for isDesktopFile(), you could use KDesktopFile::isDesktopFile(), also the isDir() check could come before, a folder cannot be a desktop file

Battery.cxx:
* It's using Solid/Power API that has never been released (and probably never will be..)
* you assume remainingTime() == -1 to be "when on AC", I'm not sure this assumption holds
* secsToHM looks not ideal form an i18n perspective

CMakeLists.txt:
* No project name set

ConfigureDesktopDialog.cxx:
* instead of QOverload or old syntax you can static_cast<void(KUrlRequester::*)(const QString &)>(&KUrlRequester::returnPressed)

ConfigureDesktopDialog.hxx:
* Reason for *returning* const &?

desktop.cxx:
* Missing AA_UseHighDpiPixmaps attribute

DesktopWidget.cxx:
* window type NET::Desktop should imply NET::KeepBelow
* The list of applets is completely hardcoded and makes assumptions in the class (for "Weather" create WeatherApplet) instead of using some plugin/introspection mechanism

LockLogout.cxx:
* Use QDBus instead of calling dbus-send command-line tool or xdg-screensaver

QuickLaunch.cxx:
* Similar issues as AppMenu.cxx
* Internet browser assumes kde.org is start page

StartMenu.cxx:
* Similar issues as LockLogout.cxx

SysTray.cxx:
* The fill function is also awfully hardcoded

General (UI) observations:
* Qt scales it quite well for high-dpi, however the application doesn't announce high dpi support leading to blurry icons, especially tray icons
* The CPU indicator in the panel cannot be disabled and is distracting
* There's lots of I18N substitution errors all over the place (I18N_ARGUMENT_MISSING)
* The "device notifier" lists *all* devices (not just removables) and isn't scrollable if there are lots of devices, network also doesn't seem scrollable
* No battery applet, icon just opens KCM
* "Bluetooth is not operational", why show the icon then
* Notifications doesnt handle multiple incoming ones well (just stacks dialogs ontop of each other)
* Start button breaks Fitt's law (I cannot open it by janking the cursor into the lower-left corner)
* No multi-screen support whatsoever
* I like the background color selector combination of ComboBox and "custom"
* Often uses QString instead of enums
* Task Bar does not announce "iconified geometries" to KWin thus minimize animations go to the wrong place (centre of screen usually)
* The coding style and naming practises do not follow "modern" KDE Frameworks guidelines

Cheers
Kai Uwe


More information about the kde-core-devel mailing list