D22034: Introduce ContainmentLayoutManager QML plugin
This revision was automatically updated to reflect the committed changes. Closed by commit R120:f59aecbcfec6: Introduce ContainmentLayoutManager QML plugin (authored by mart). REPOSITORY R120 Plasma Workspace CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D22034?vs=61987&id=62268 REVISION DETAIL https://phabricator.kde.org/D22034 AFFECTED FILES components/CMakeLists.txt components/containmentlayoutmanager/CMakeLists.txt components/containmentlayoutmanager/abstractlayoutmanager.cpp components/containmentlayoutmanager/abstractlayoutmanager.h components/containmentlayoutmanager/appletcontainer.cpp components/containmentlayoutmanager/appletcontainer.h components/containmentlayoutmanager/appletslayout.cpp components/containmentlayoutmanager/appletslayout.h components/containmentlayoutmanager/configoverlay.cpp components/containmentlayoutmanager/configoverlay.h components/containmentlayoutmanager/containmentlayoutmanagerplugin.cpp components/containmentlayoutmanager/containmentlayoutmanagerplugin.h components/containmentlayoutmanager/gridlayoutmanager.cpp components/containmentlayoutmanager/gridlayoutmanager.h components/containmentlayoutmanager/itemcontainer.cpp components/containmentlayoutmanager/itemcontainer.h components/containmentlayoutmanager/qml/BasicAppletContainer.qml components/containmentlayoutmanager/qml/ConfigOverlayWithHandles.qml components/containmentlayoutmanager/qml/PlaceHolder.qml components/containmentlayoutmanager/qml/private/BasicResizeHandle.qml components/containmentlayoutmanager/qml/qmldir components/containmentlayoutmanager/resizehandle.cpp components/containmentlayoutmanager/resizehandle.h To: mart, #plasma, davidedmundson Cc: davidedmundson, plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
D22034: Introduce ContainmentLayoutManager QML plugin
mart updated this revision to Diff 61987. mart marked an inline comment as done. mart added a comment. - set the item container directly REPOSITORY R120 Plasma Workspace CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D22034?vs=61976&id=61987 BRANCH mart/containmentlayoutmanager REVISION DETAIL https://phabricator.kde.org/D22034 AFFECTED FILES components/CMakeLists.txt components/containmentlayoutmanager/CMakeLists.txt components/containmentlayoutmanager/abstractlayoutmanager.cpp components/containmentlayoutmanager/abstractlayoutmanager.h components/containmentlayoutmanager/appletcontainer.cpp components/containmentlayoutmanager/appletcontainer.h components/containmentlayoutmanager/appletslayout.cpp components/containmentlayoutmanager/appletslayout.h components/containmentlayoutmanager/configoverlay.cpp components/containmentlayoutmanager/configoverlay.h components/containmentlayoutmanager/containmentlayoutmanagerplugin.cpp components/containmentlayoutmanager/containmentlayoutmanagerplugin.h components/containmentlayoutmanager/gridlayoutmanager.cpp components/containmentlayoutmanager/gridlayoutmanager.h components/containmentlayoutmanager/itemcontainer.cpp components/containmentlayoutmanager/itemcontainer.h components/containmentlayoutmanager/qml/BasicAppletContainer.qml components/containmentlayoutmanager/qml/ConfigOverlayWithHandles.qml components/containmentlayoutmanager/qml/PlaceHolder.qml components/containmentlayoutmanager/qml/private/BasicResizeHandle.qml components/containmentlayoutmanager/qml/qmldir components/containmentlayoutmanager/resizehandle.cpp components/containmentlayoutmanager/resizehandle.h To: mart, #plasma Cc: davidedmundson, plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
D22034: Introduce ContainmentLayoutManager QML plugin
davidedmundson added inline comments. INLINE COMMENTS > configoverlay.h:67 > +private: > +void setItemContainer(ItemContainer *container); > + Given ItemContainer instantiates this object (or a subclass therefore) can we make this public and call it from ItemContainer::setConfigOverlayVisible Then we get rid of one of the two searches through parents. REPOSITORY R120 Plasma Workspace REVISION DETAIL https://phabricator.kde.org/D22034 To: mart, #plasma Cc: davidedmundson, plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
D22034: Introduce ContainmentLayoutManager QML plugin
mart marked an inline comment as done. mart added inline comments. INLINE COMMENTS > davidedmundson wrote in itemcontainer.cpp:438 > If we want to use this in the panel/wherever we need ItemContainer to forward > the full min/preferred/max sizes (with margins added) and have the > LayoutManager be the one that takes this information and resizes the > container. > > IMHO it shouldn't resize itself, it should just emit sizeHintChanged > > Then in AbstractLayoutManager we have a connect that does the current: > > releaseSpace(item) > resizeFromSizeHints(item) > positionItem(item) > > It then gives full absolute control to the manager, rather than it doing part > and then the item doing part. same code now moved in the layoutmanager REPOSITORY R120 Plasma Workspace REVISION DETAIL https://phabricator.kde.org/D22034 To: mart, #plasma Cc: davidedmundson, plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
D22034: Introduce ContainmentLayoutManager QML plugin
mart updated this revision to Diff 61976. mart marked 3 inline comments as done. mart added a comment. - no manual parenting - move the reaction to size hints in layoutmanager REPOSITORY R120 Plasma Workspace CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D22034?vs=61973&id=61976 BRANCH mart/containmentlayoutmanager REVISION DETAIL https://phabricator.kde.org/D22034 AFFECTED FILES components/CMakeLists.txt components/containmentlayoutmanager/CMakeLists.txt components/containmentlayoutmanager/abstractlayoutmanager.cpp components/containmentlayoutmanager/abstractlayoutmanager.h components/containmentlayoutmanager/appletcontainer.cpp components/containmentlayoutmanager/appletcontainer.h components/containmentlayoutmanager/appletslayout.cpp components/containmentlayoutmanager/appletslayout.h components/containmentlayoutmanager/configoverlay.cpp components/containmentlayoutmanager/configoverlay.h components/containmentlayoutmanager/containmentlayoutmanagerplugin.cpp components/containmentlayoutmanager/containmentlayoutmanagerplugin.h components/containmentlayoutmanager/gridlayoutmanager.cpp components/containmentlayoutmanager/gridlayoutmanager.h components/containmentlayoutmanager/itemcontainer.cpp components/containmentlayoutmanager/itemcontainer.h components/containmentlayoutmanager/qml/BasicAppletContainer.qml components/containmentlayoutmanager/qml/ConfigOverlayWithHandles.qml components/containmentlayoutmanager/qml/PlaceHolder.qml components/containmentlayoutmanager/qml/private/BasicResizeHandle.qml components/containmentlayoutmanager/qml/qmldir components/containmentlayoutmanager/resizehandle.cpp components/containmentlayoutmanager/resizehandle.h To: mart, #plasma Cc: davidedmundson, plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
D22034: Introduce ContainmentLayoutManager QML plugin
mart updated this revision to Diff 61973. mart added a comment. - better manage locked plasmoid logic REPOSITORY R120 Plasma Workspace CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D22034?vs=61966&id=61973 BRANCH mart/containmentlayoutmanager REVISION DETAIL https://phabricator.kde.org/D22034 AFFECTED FILES components/CMakeLists.txt components/containmentlayoutmanager/CMakeLists.txt components/containmentlayoutmanager/abstractlayoutmanager.cpp components/containmentlayoutmanager/abstractlayoutmanager.h components/containmentlayoutmanager/appletcontainer.cpp components/containmentlayoutmanager/appletcontainer.h components/containmentlayoutmanager/appletslayout.cpp components/containmentlayoutmanager/appletslayout.h components/containmentlayoutmanager/configoverlay.cpp components/containmentlayoutmanager/configoverlay.h components/containmentlayoutmanager/containmentlayoutmanagerplugin.cpp components/containmentlayoutmanager/containmentlayoutmanagerplugin.h components/containmentlayoutmanager/gridlayoutmanager.cpp components/containmentlayoutmanager/gridlayoutmanager.h components/containmentlayoutmanager/itemcontainer.cpp components/containmentlayoutmanager/itemcontainer.h components/containmentlayoutmanager/qml/BasicAppletContainer.qml components/containmentlayoutmanager/qml/ConfigOverlayWithHandles.qml components/containmentlayoutmanager/qml/PlaceHolder.qml components/containmentlayoutmanager/qml/private/BasicResizeHandle.qml components/containmentlayoutmanager/qml/qmldir components/containmentlayoutmanager/resizehandle.cpp components/containmentlayoutmanager/resizehandle.h To: mart, #plasma Cc: davidedmundson, plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
D22034: Introduce ContainmentLayoutManager QML plugin
davidedmundson added inline comments.
INLINE COMMENTS
> mart wrote in itemcontainer.cpp:370
> I may be wrong but I think if one would do
> AppletsLayout {
>
> QtObject {}
>
> }
>
> that internal object wouldn't be parented and stay dangling otherwise?
AFAIK, QML created objects don't use QObject parent.
Instead there's a ref counting thing in the JS engine.
> mart wrote in itemcontainer.cpp:438
> This may be not necessary, depends the behavior we want.
> The test i did was with the system monitor, having it auto resize when one
> adds a sensor seemed to look better...
> It depends from usability pov, if instead by design we say the size the user
> manually put is more important and should be maintained if possible, then
> this part would go.
If we want to use this in the panel/wherever we need ItemContainer to forward
the full min/preferred/max sizes (with margins added) and have the
LayoutManager be the one that takes this information and resizes the container.
IMHO it shouldn't resize itself, it should just emit sizeHintChanged
Then in AbstractLayoutManager we have a connect that does the current:
releaseSpace(item)
resizeFromSizeHints(item)
positionItem(item)
It then gives full absolute control to the manager, rather than it doing part
and then the item doing part.
REPOSITORY
R120 Plasma Workspace
REVISION DETAIL
https://phabricator.kde.org/D22034
To: mart, #plasma
Cc: davidedmundson, plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2,
ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg,
abetts, sebas, apol, mart
D22034: Introduce ContainmentLayoutManager QML plugin
mart updated this revision to Diff 61966. mart marked 5 inline comments as done. mart added a comment. - singal when can't resize - make resize work with items that update hints while resizing - use setPosition and setSize where possible - make sure to connect to busy indicator - don't try to assign space when the layout has an invalid size - adress some comments REPOSITORY R120 Plasma Workspace CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D22034?vs=61786&id=61966 BRANCH mart/containmentlayoutmanager REVISION DETAIL https://phabricator.kde.org/D22034 AFFECTED FILES components/CMakeLists.txt components/containmentlayoutmanager/CMakeLists.txt components/containmentlayoutmanager/abstractlayoutmanager.cpp components/containmentlayoutmanager/abstractlayoutmanager.h components/containmentlayoutmanager/appletcontainer.cpp components/containmentlayoutmanager/appletcontainer.h components/containmentlayoutmanager/appletslayout.cpp components/containmentlayoutmanager/appletslayout.h components/containmentlayoutmanager/configoverlay.cpp components/containmentlayoutmanager/configoverlay.h components/containmentlayoutmanager/containmentlayoutmanagerplugin.cpp components/containmentlayoutmanager/containmentlayoutmanagerplugin.h components/containmentlayoutmanager/gridlayoutmanager.cpp components/containmentlayoutmanager/gridlayoutmanager.h components/containmentlayoutmanager/itemcontainer.cpp components/containmentlayoutmanager/itemcontainer.h components/containmentlayoutmanager/qml/BasicAppletContainer.qml components/containmentlayoutmanager/qml/ConfigOverlayWithHandles.qml components/containmentlayoutmanager/qml/PlaceHolder.qml components/containmentlayoutmanager/qml/private/BasicResizeHandle.qml components/containmentlayoutmanager/qml/qmldir components/containmentlayoutmanager/resizehandle.cpp components/containmentlayoutmanager/resizehandle.h To: mart, #plasma Cc: davidedmundson, plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
D22034: Introduce ContainmentLayoutManager QML plugin
mart added inline comments.
INLINE COMMENTS
> davidedmundson wrote in abstractlayoutmanager.h:44
> I don't fully understand the split between AbstractLayoutManager and
> GridLayoutManager when AbstractLayoutManager effectively enforces a grid by
> having cell sizes.
in the end it needed cell sizes to be directly exposed to qml.
tough the idea was to leave space for eventual future different layouts like
linear, or still grid but with a different strategy and so on
> davidedmundson wrote in itemcontainer.cpp:370
> why?
I may be wrong but I think if one would do
AppletsLayout {
QtObject {}
}
that internal object wouldn't be parented and stay dangling otherwise?
> davidedmundson wrote in itemcontainer.cpp:438
> why?
>
> We're above the minimum size, the user could have resized it smaller than this
This may be not necessary, depends the behavior we want.
The test i did was with the system monitor, having it auto resize when one adds
a sensor seemed to look better...
It depends from usability pov, if instead by design we say the size the user
manually put is more important and should be maintained if possible, then this
part would go.
> davidedmundson wrote in BasicResizeHandle.qml:26
> ResizeHandle2?
dead code of old prototypes
> davidedmundson wrote in resizehandle.cpp:42
> we monitor the direct parent, but ConfigOverlay could be anywhere on the
> ancestory tree.
This will catch it if the overlay is an ancestor..
what it doesn't support is dynamic parent changes somewhere in the tree higher
than direct parent.
monitoring every parent of parents and dynamically rebuilding the connection
chain when one of them change, seems a bit.. overkill?
not sure how to do this in another way...
REPOSITORY
R120 Plasma Workspace
REVISION DETAIL
https://phabricator.kde.org/D22034
To: mart, #plasma
Cc: davidedmundson, plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2,
ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg,
abetts, sebas, apol, mart
D22034: Introduce ContainmentLayoutManager QML plugin
davidedmundson added a comment.
Concept makes sense. I mostly like the layering and architecture, except for
all the cases where we end up searching up the hierarchy to find objects. That
doesn't seem quite as nicely layered.
I'm not finished, it's a big patch - but I thought I should submit what I
have for now.
INLINE COMMENTS
> abstractlayoutmanager.cpp:110
> +QRectF candidate = candidateGeometry(item);
> +item->setX(candidate.x());
> +item->setY(candidate.y());
setPosition
setSize
> abstractlayoutmanager.h:44
> + */
> +QSizeF cellAlignedContainingSize(const QSizeF &size) const;
> +
I don't fully understand the split between AbstractLayoutManager and
GridLayoutManager when AbstractLayoutManager effectively enforces a grid by
having cell sizes.
> appletcontainer.cpp:32
> +{
> +connect(this, &AppletContainer::contentItemChanged, this, [this]() {
> +if (m_appletItem) {
How do we know this happens after the busy component is set?
> itemcontainer.cpp:364
> +
> +QQuickItem *item;
> +for (auto *o : m_contentData) {
scope this in the loop
> itemcontainer.cpp:370
> +} else {
> +o->setParent(this);
> +}
why?
> itemcontainer.cpp:375
> +// Search for the Layout attached property
> +// Qt6: this should become public api
> +for (auto *o : children()) {
no point just wishing, we need to open a bug report at least
> itemcontainer.cpp:438
> +if (newPreferredHeight > height()) {
> +setHeight(layout()->cellHeight() * ceil(newPreferredHeight /
> layout()->cellHeight()));
> +changed = true;
why?
We're above the minimum size, the user could have resized it smaller than this
> BasicResizeHandle.qml:26
> +ContainmentLayoutManager.ResizeHandle {
> +resizeCorner: ContainmentLayoutManager.ResizeHandle2.Right
> +
ResizeHandle2?
> resizehandle.cpp:42
> +
> +connect(this, &QQuickItem::parentChanged, this, [this]() {
> +QQuickItem *candidate = parentItem();
we monitor the direct parent, but ConfigOverlay could be anywhere on the
ancestory tree.
REPOSITORY
R120 Plasma Workspace
REVISION DETAIL
https://phabricator.kde.org/D22034
To: mart, #plasma
Cc: davidedmundson, plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2,
ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg,
abetts, sebas, apol, mart
D22034: Introduce ContainmentLayoutManager QML plugin
mart updated this revision to Diff 61786. mart added a comment. - never auto resize while on edit mode - try to restore size as soon as possible REPOSITORY R120 Plasma Workspace CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D22034?vs=61642&id=61786 BRANCH mart/containmentlayoutmanager REVISION DETAIL https://phabricator.kde.org/D22034 AFFECTED FILES components/CMakeLists.txt components/containmentlayoutmanager/CMakeLists.txt components/containmentlayoutmanager/abstractlayoutmanager.cpp components/containmentlayoutmanager/abstractlayoutmanager.h components/containmentlayoutmanager/appletcontainer.cpp components/containmentlayoutmanager/appletcontainer.h components/containmentlayoutmanager/appletslayout.cpp components/containmentlayoutmanager/appletslayout.h components/containmentlayoutmanager/configoverlay.cpp components/containmentlayoutmanager/configoverlay.h components/containmentlayoutmanager/containmentlayoutmanagerplugin.cpp components/containmentlayoutmanager/containmentlayoutmanagerplugin.h components/containmentlayoutmanager/gridlayoutmanager.cpp components/containmentlayoutmanager/gridlayoutmanager.h components/containmentlayoutmanager/itemcontainer.cpp components/containmentlayoutmanager/itemcontainer.h components/containmentlayoutmanager/qml/BasicAppletContainer.qml components/containmentlayoutmanager/qml/ConfigOverlayWithHandles.qml components/containmentlayoutmanager/qml/PlaceHolder.qml components/containmentlayoutmanager/qml/private/BasicResizeHandle.qml components/containmentlayoutmanager/qml/qmldir components/containmentlayoutmanager/resizehandle.cpp components/containmentlayoutmanager/resizehandle.h To: mart, #plasma Cc: plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
D22034: Introduce ContainmentLayoutManager QML plugin
mart updated this revision to Diff 61642. mart added a comment. - manage differently screen resize and other kind of resizes - fix snapping on RTL and BTT positioning REPOSITORY R120 Plasma Workspace CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D22034?vs=61576&id=61642 BRANCH mart/containmentlayoutmanager REVISION DETAIL https://phabricator.kde.org/D22034 AFFECTED FILES components/CMakeLists.txt components/containmentlayoutmanager/CMakeLists.txt components/containmentlayoutmanager/abstractlayoutmanager.cpp components/containmentlayoutmanager/abstractlayoutmanager.h components/containmentlayoutmanager/appletcontainer.cpp components/containmentlayoutmanager/appletcontainer.h components/containmentlayoutmanager/appletslayout.cpp components/containmentlayoutmanager/appletslayout.h components/containmentlayoutmanager/configoverlay.cpp components/containmentlayoutmanager/configoverlay.h components/containmentlayoutmanager/containmentlayoutmanagerplugin.cpp components/containmentlayoutmanager/containmentlayoutmanagerplugin.h components/containmentlayoutmanager/gridlayoutmanager.cpp components/containmentlayoutmanager/gridlayoutmanager.h components/containmentlayoutmanager/itemcontainer.cpp components/containmentlayoutmanager/itemcontainer.h components/containmentlayoutmanager/qml/BasicAppletContainer.qml components/containmentlayoutmanager/qml/ConfigOverlayWithHandles.qml components/containmentlayoutmanager/qml/PlaceHolder.qml components/containmentlayoutmanager/qml/private/BasicResizeHandle.qml components/containmentlayoutmanager/qml/qmldir components/containmentlayoutmanager/resizehandle.cpp components/containmentlayoutmanager/resizehandle.h To: mart, #plasma Cc: plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
D22034: Introduce ContainmentLayoutManager QML plugin
mart updated this revision to Diff 61576. mart added a comment. - allow a limited auto expanding for applets - click on empty areas always closes edit mode - wire up maximum size - start on a new logic for default sizes - event comppress sizehints - take minimum size into account - new behavior for resize handles REPOSITORY R120 Plasma Workspace CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D22034?vs=60459&id=61576 BRANCH mart/containmentlayoutmanager REVISION DETAIL https://phabricator.kde.org/D22034 AFFECTED FILES components/CMakeLists.txt components/containmentlayoutmanager/CMakeLists.txt components/containmentlayoutmanager/abstractlayoutmanager.cpp components/containmentlayoutmanager/abstractlayoutmanager.h components/containmentlayoutmanager/appletcontainer.cpp components/containmentlayoutmanager/appletcontainer.h components/containmentlayoutmanager/appletslayout.cpp components/containmentlayoutmanager/appletslayout.h components/containmentlayoutmanager/configoverlay.cpp components/containmentlayoutmanager/configoverlay.h components/containmentlayoutmanager/containmentlayoutmanagerplugin.cpp components/containmentlayoutmanager/containmentlayoutmanagerplugin.h components/containmentlayoutmanager/gridlayoutmanager.cpp components/containmentlayoutmanager/gridlayoutmanager.h components/containmentlayoutmanager/itemcontainer.cpp components/containmentlayoutmanager/itemcontainer.h components/containmentlayoutmanager/qml/BasicAppletContainer.qml components/containmentlayoutmanager/qml/ConfigOverlayWithHandles.qml components/containmentlayoutmanager/qml/PlaceHolder.qml components/containmentlayoutmanager/qml/private/BasicResizeHandle.qml components/containmentlayoutmanager/qml/qmldir components/containmentlayoutmanager/resizehandle.cpp components/containmentlayoutmanager/resizehandle.h To: mart, #plasma Cc: plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
