Review Request 119944: Remove UI assumptions about the alternatives dialog

Marco Martin notmart at gmail.com
Tue Aug 26 17:53:24 UTC 2014


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/119944/#review65296
-----------------------------------------------------------

Ship it!


instancing the Dialog from QML rather than having it created in c++ is something i wanted to do, so yay
(note this should be pushed only together https://git.reviewboard.kde.org/r/119946)

- Marco Martin


On Aug. 26, 2014, 5:35 p.m., Aaron J. Seigo wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/119944/
> -----------------------------------------------------------
> 
> (Updated Aug. 26, 2014, 5:35 p.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Repository: plasma-workspace
> 
> 
> Description
> -------
> 
> This changes AlternativesDialog to AlternativesHelper that only exports the necessary functionality, and puts the onus on the shell package to provide the proper UI. The alternatives QML is expected to provide a bool visibile property with a visibleChanged signal to allow proper cleanup. This will be documented in the plasmashell documentation that I am also working on.
> 
> This happens to remove the need for plasmaquick's Dialog class in plasmashell. For those who haven't noticed: libplasmaquick is not providing binary compatibility guarantees and is accessed by *private headers copied over into repositories*; seeing as one package is released with KDE Frameworks and the other Plasma Workspace, this is obviously a binary compatibility disaster just waiting to happen and quite obviously violates the contract between Frameworks and Workspace being different products. This patch set also introduces the need to use plasmaquick/ in the #include directives so it is easy to see where the trouble spots are. 
> 
> The other motivation for this change is that providing an *applet-positioned dialog* is a gross assumption about the hardware form factor plasmashell is running on. The entire point of having plasmashell was to have a single shell that can be used on different form factors and with different shell packages that dictate the results, yet such assumptions have crept in one by one.
> 
> This patch set is one small step to addressing these issues which currently prevent plasmashell from being a serious candidate for use on non-desktop/laptop form factors.
> 
> 
> Diffs
> -----
> 
>   shell/interactiveconsole.cpp e7a3e85e0119ea6fa22a293ec226ae23005e9b2c 
>   shell/main.cpp e34578d8142e95362734fdd4db9d06da7fc02b20 
>   shell/osd.cpp a2bee29f400f8c4aca690205f55699bae185b647 
>   shell/panelconfigview.h 02e1264572d4b9b744cd6d4aaf1df694f3e4aa6d 
>   shell/panelview.h bd1643813bb182c5faaf4682fbd4b7adb61979a7 
>   shell/plasmaquick/dialog.h 6759a6ea37472a71e291e3bfaab0d7a82afba323 
>   shell/shellcorona.h bb9f0c1110887b372a4e084a64abb0fabfa1bc5b 
>   shell/shellcorona.cpp 2b8389ee54b74bfe84ad685d145f9dabb5b24a8f 
>   shell/CMakeLists.txt 700d08b419918b9863c44691473cd4f9cae00d86 
>   shell/alternativesdialog.h 195b69a304d7708afbe127928b2a0e129880f281 
>   shell/alternativesdialog.cpp 56d686ded958bfa7de2579e4c8dce04d3771d615 
>   shell/containmentconfigview.h 8c6c2a2444c0e031cb21876a56ae3b861b01e902 
>   shell/containmentconfigview.cpp acfa6b24f11beed00b81b557b39dcf6f39eedcf6 
>   shell/desktopview.h 620d6df5a2fa516b9d9fc59184f2d9c7669efc5f 
> 
> Diff: https://git.reviewboard.kde.org/r/119944/diff/
> 
> 
> Testing
> -------
> 
> Run with a modified desktop shell package and alternatives come up as expected and the QmlObjects are cleaned up appropriately as well.
> 
> 
> Thanks,
> 
> Aaron J. Seigo
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20140826/faf40ab4/attachment.html>


More information about the Plasma-devel mailing list