konstantinshtepa added a comment.
In https://phabricator.kde.org/D4204#80553, @davidedmundson wrote:
> Though really any max > min on the client is undefined behaviour, so it's
hard to say any is "right".
You are right. It's undefined behaviour. At current state there is a hole
konstantinshtepa added inline comments.
INLINE COMMENTS
> davidedmundson wrote in AppletAppearance.qml:445
> Edit, maybe it won't - that's why you have the separate Binding.
>
> However changing this to:
> minimumWidth: Math.min(minimumSize.width, maximumSize.width);
>
> for all 4
>
> would
konstantinshtepa added a comment.
In https://phabricator.kde.org/D4204#80142, @mart wrote:
> what i would like to have logically splitted is the management of the
floating property (and having positionItem()/releasePosition used around)
>
> and on the other hand the signal handlers
konstantinshtepa added inline comments.
INLINE COMMENTS
> davidedmundson wrote in AppletAppearance.qml:445
> Edit, maybe it won't - that's why you have the separate Binding.
>
> However changing this to:
> minimumWidth: Math.min(minimumSize.width, maximumSize.width);
>
> for all 4
>
> would
konstantinshtepa added inline comments.
INLINE COMMENTS
> davidedmundson wrote in AppletAppearance.qml:445
> Edit, maybe it won't - that's why you have the separate Binding.
>
> However changing this to:
> minimumWidth: Math.min(minimumSize.width, maximumSize.width);
>
> for all 4
>
> would
konstantinshtepa added inline comments.
INLINE COMMENTS
> davidedmundson wrote in AppletAppearance.qml:101
> this is broken.
>
> if I'm an applet and do:
> Plasmoid.Layout.maximumWidth = 50
>
> this appletItem.maximumWidth == 58 (assuming 4px margins)
> which is correct
>
> Now if I do:
>
>
davidedmundson added inline comments.
INLINE COMMENTS
> AppletAppearance.qml:445
> +
> property int minimumWidth: minimumSize.width;
> property int minimumHeight: minimumSize.height;
Edit, maybe it won't - that's why you have the separate Binding.
However
davidedmundson added inline comments.
INLINE COMMENTS
> AppletAppearance.qml:101
> +if (minimumWidth > maximumWidth)
> +maximumWidth = minimumWidth;
> +if (width < minimumWidth) {
this is broken.
if I'm an applet and do:
Plasmoid.Layout.maximumWidth = 50
this
davidedmundson added inline comments.
INLINE COMMENTS
> mart wrote in AppletAppearance.qml:50
> mouseListener just has a simple anchors.fill:parent to this, so makes
> innerWidth/innerHeight redundant as they are the same as the parent?
It did. It doesn't after this patch.
It's detached so
mart added a comment.
In https://phabricator.kde.org/D4204#80123, @konstantinshtepa wrote:
> In https://phabricator.kde.org/D4204#80106, @mart wrote:
>
> > can this be splitted in multiple reviews/commits?
>
>
> It can be splitted into multiple commits inside one branch so you
konstantinshtepa added a comment.
In https://phabricator.kde.org/D4204#80106, @mart wrote:
> can this be splitted in multiple reviews/commits?
It can be splitted into multiple commits inside one branch so you can view
what exactly fix what. But multiple reviews based on master? I
mart added a comment.
can this be splitted in multiple reviews/commits?
REPOSITORY
R119 Plasma Desktop
REVISION DETAIL
https://phabricator.kde.org/D4204
EMAIL PREFERENCES
https://phabricator.kde.org/settings/panel/emailpreferences/
To: konstantinshtepa, #plasma
Cc: mart,
konstantinshtepa added a comment.
A detailed explanation of bugs, fixes and mechanics changes
===
Before anything else I'd like to explain why I need this patch. At winter
holidays I had a free time and decided to rewrite
davidedmundson added inline comments.
INLINE COMMENTS
> AppletAppearance.qml:59
> +appletContainer.anchors.leftMargin +
> +appletContainer.anchors.rightMargin - handleWidth);
> +}
why are you including handleWidth here?
>From what I can see
konstantinshtepa added a comment.
WARNING: Found a bug with plasmoid don't free all his space in LayoutManager
when changed from compactRepresentation to fullRepresentation. Don't push this
patch before I fix this bug.
REPOSITORY
R119 Plasma Desktop
REVISION DETAIL
konstantinshtepa added a comment.
In https://phabricator.kde.org/D4204#78701, @davidedmundson wrote:
> That's quite a huge patch.
> Can you give a much bigger explanation on exactly what it's doing and why.
> "Fixes bug N" doesn't really explain what the original problem was.
davidedmundson added a comment.
That's quite a huge patch.
Can you give a much bigger explanation on exactly what it's doing and why.
"Fixes bug N" doesn't really explain what the original problem was.
INLINE COMMENTS
> AppletAppearance.qml:61
>
> -property int minimumHeight:
17 matches
Mail list logo
