Figure out what to do with IconLabel

Currently, overriding the contentItem of e.g. Button entails either quite a bit of work from the user (text and icon creation, positioning, icon colouring, etc.) or copying the style's implementation, which uses the private IconLabel type:

import QtQuick 2.12
import QtQuick.Controls 2.12
import QtQuick.Controls.impl 2.12
import QtQuick.Templates 2.12 as T

T.Button {
    id: control

    implicitWidth: Math.max(implicitBackgroundWidth + leftInset + rightInset,
                            implicitContentWidth + leftPadding + rightPadding)
    implicitHeight: Math.max(implicitBackgroundHeight + topInset + bottomInset,
                             implicitContentHeight + topPadding + bottomPadding)

    padding: 6
    horizontalPadding: padding + 2
    spacing: 6

    icon.width: 24
    icon.height: 24
    icon.color: control.checked || control.highlighted ? control.palette.brightText :
                control.flat && !control.down ? (control.visualFocus ? control.palette.highlight : control.palette.windowText) : control.palette.buttonText

    contentItem: IconLabel {
        spacing: control.spacing
        mirrored: control.mirrored
        display: control.display

        icon: control.icon
        text: control.text
        font: control.font
        color: control.checked || control.highlighted ? control.palette.brightText :
               control.flat && !control.down ? (control.visualFocus ? control.palette.highlight : control.palette.windowText) : control.palette.buttonText
    }

    background: Rectangle {
        implicitWidth: 100
        implicitHeight: 40
        visible: !control.flat || control.down || control.checked || control.highlighted
        color: Color.blend(control.checked || control.highlighted ? control.palette.dark : control.palette.button,
                                                                    control.palette.mid, control.down ? 0.5 : 0.0)
        border.color: control.palette.highlight
        border.width: control.visualFocus ? 2 : 0
    }
}

Copying code as a basis for customising a control is (at least currently) an inescapable part of working with Controls, so we should make sure that at least the Default style uses as little internal/private API as possible (preferably none at all). That way, the user has one style where they can safely copy code without worrying about it breaking in the next release. To achieve this, we need to use public types in the QML style implementations.

We have a couple of solutions:

Make IconLabel public

J-P had this to say:

IconLabel, on the other hand... I'm not sure I'd make it public at this point. It's not really a control, and was implemented as an internal helper type just to workaround typical QtQuick core limitations. I'd rather try to fix those limitations instead... For example, to start with, positioners are unusable as building blocks for controls, because they use items' explicit sizes to calculate their own implicit size. Being able to change the orientation of a linear positioner would be also nice, even though this can be worked around with Grid, which apparently even provides item alignment since version x.6.

To expand on what the implicit vs explicit size issue is, I'll quote my chat history from when I asked him (with paste links replaced with attachments since they've long since expired):

[12:36:43] <micurtis> btw, was it implicit size that made the row grow? [p8txoosbs]
[12:36:59] <micurtis> i'm trying to reproduce it. took a look through our chat history but couldn't find a mention of it
[12:38:34] <jpnurmi> hmm
[12:40:29] <jpnurmi> for example, in case of a button you would have a row that has an image and a label
[12:41:06] <jpnurmi> when the button is resized, the width of the label changes
[12:41:23] <jpnurmi> the problem is that the implicit width of the row changes too, when the label resizes itself
[12:41:58] <jpnurmi> the original implicit width is lost. reseting the width of the button does not shrink the button back to its original size
[12:42:08] <jpnurmi> or changing the text doesn't make the button resize as appropriate
[12:42:23] <jpnurmi> the size keeps growing and growing, but never shrinks
[12:50:05] <jpnurmi> [pcvkjjt4h]
[12:50:28] <jpnurmi> ^ in our case, the implicit size of the row should not change when you resize the window
[12:52:08] <jpnurmi> Row { implicitSizeMode: Row.SomethingThatDescribesThatTheImplicitSizeOfTheRowIsCalculatedBasedOnTheImplicitSizeOfTheChildren }
[12:55:50] <micurtis> haha
[12:56:30] <micurtis> "the implicit size of the row should not change when you resize the window" because Text's implicitWidth never changed?
[12:58:37] <micurtis> "the original implicit width is lost. reseting the width of the button does not shrink the button back to its original size" do you mean by literally doing button.width = undefined?
[12:58:52] <micurtis> if so, when does that happen? (just trying to understand the whole problem)
[13:05:21] <jpnurmi> "item.width = undefined" calls QQuickItem::resetWidth() so width becomes implicitWidth
[13:05:29] <jpnurmi> just as an example
[13:05:59] <jpnurmi> better example: a control that does anchor.centerIn: parent
[13:06:30] <jpnurmi> no explicit size specified, so the control always follows its implicit size
[13:06:59] <jpnurmi> set "something very long text" for a button, and the button grows
[13:07:18] <jpnurmi> change the text to something "short" and it must shrink
[13:08:45] <jpnurmi> even something as simple as this doesn't work with positioners: [prfbe1vym]
[13:09:14] <jpnurmi> it doesn't pick the implicit widths of the labels, but uses the explicit widths instead
[13:09:59] <jpnurmi> since the explicit width depends on the parent, the positioner is messed up

• p8txoosbs.txt
• pcvkjjt4h.txt
• prfbe1vym.txt

Which leads us on to the second option...

Make IconImage public, fix positioners, etc.

Make it possible to do what IconLabel does by using Qt Quick primitives. I imagine this would look something like:

import QtQuick 2.12
import QtQuick.Controls 2.12
import QtQuick.Controls.impl 2.12
import QtQuick.Templates 2.12 as T

T.Button {
    id: control

    implicitWidth: Math.max(implicitBackgroundWidth + leftInset + rightInset,
                            implicitContentWidth + leftPadding + rightPadding)
    implicitHeight: Math.max(implicitBackgroundHeight + topInset + bottomInset,
                             implicitContentHeight + topPadding + bottomPadding)

    padding: 6
    horizontalPadding: padding + 2
    spacing: 6

    icon.width: 24
    icon.height: 24
    icon.color: control.checked || control.highlighted ? control.palette.brightText :
                control.flat && !control.down ? (control.visualFocus ? control.palette.highlight : control.palette.windowText) : control.palette.buttonText

    contentItem: Grid {
        spacing: control.spacing
        // Should be handled for us by LayoutMirroring.enabled?
        //mirrored: control.mirrored
        columns: control.display === AbstractButton.TextBesideIcon ? 2 : 1
        rows: control.display === AbstractButton.TextUnderIcon ? 2 : 1
        horizontalItemAlignment: Grid.AlignHCenter
        verticalItemAlignment: Grid.AlignVCenter

        // Can't use ids here, otherwise we could set this in e.g. IconImage and refer to it in Label
        readonly property color color: control.checked || control.highlighted ? control.palette.brightText :
            control.flat && !control.down ? (control.visualFocus ? control.palette.highlight : control.palette.windowText) : control.palette.buttonText

        IconImage {
            icon: control.icon
            color: parent.color
        }

        Label {
            text: control.text
            font: control.font
            color: parent.color
        }
    }

    background: Rectangle {
        implicitWidth: 100
        implicitHeight: 40
        visible: !control.flat || control.down || control.checked || control.highlighted
        color: Color.blend(control.checked || control.highlighted ? control.palette.dark : control.palette.button,
                                                                    control.palette.mid, control.down ? 0.5 : 0.0)
        border.color: control.palette.highlight
        border.width: control.visualFocus ? 2 : 0
    }
}

This still leaves other private types in the implementation (the Color singleton), but that might not be something we can avoid completely.

Issues with this approach:

• Breaks

  { Rectangle { width : 30 } ... }

  as Rectangle has an implicitWidth of 0. So it would need an opt-in property in QQuickBasePositioner to control the behaviour.

• https://doc.qt.io/qt-5/qml-qtquick-grid.html#horizontalItemAlignment-prop and friends don't behave in the way we need them to: the property refers to the position of the item within its cell, not to its position within the positioner.

Note that there was a patch to fix the positioners, but it was reverted (and I don't remember why): https://codereview.qt-project.org/c/qt/qtdeclarative/+/190028.

Conclusion

It seems that using positioners for icon + label layouting is not possible, even after the implicit size changes we would need to make. In that case, making IconLabel public seems like the best choice.

If we do make IconLabel public, we should decide (i.e. benchmark) if we want to make it derive from QQuickControl to fix QTBUG-81869. Although, one thing to keep in mind there is Vitaly's patch for palette inheritance. If that gets accepted, it sets precedence for font inheritance in QQuickItem, and if we had that, making IconLabel a control wouldn't be necessary (and would just be overhead).