-----------------------------------------------------------
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
> 
>

_______________________________________________
Plasma-devel mailing list
[email protected]
https://mail.kde.org/mailman/listinfo/plasma-devel

Reply via email to