[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-devel
Subject:    Spacing in our apps
From:       Carl Schwan <carl () carlschwan ! eu>
Date:       2023-12-17 12:21:32
Message-ID: 2236186.Hq7AAxBmiT () fedora
[Download RAW message or body]

Hi,

I'm been trying to unify a bit the usage of spacing in our apps and I'm 
noticing a difference between how we do it in QWidgets apps and QML apps.

In QtWidgets apps, we use

- pixelMetric(QStyle::PM_Layout{Left,Right,Top,Bottom}Margin) for the margins
- pixelMetric(QStyle::PM_Layout{Vertical,Horizontal}Margin) for the spacing 
between items in layout

In practice all these pixel metrics are equal to 6 pixels with Fusion, Oxygen 
and Breeze. These means that in some cases, some apps are even hardcoding 
these values in their .ui files, which is bad and we should try to avoid this.

In Kirigami apps, we use:

- Kirigami.Units.smallSpacing = 4 pixels
- Kirigami.Units.mediumSpacing = 6 pixels
- Kirigami.Units.largeSpacing = 8 pixels

In most cases, smallSpacing and largeSpacing are used as mediumSpacing was 
introduced later on, so it doesn't match the values from QtWidgets apps. Worse 
we don't really have clear guidelines when to use small or large spacing, so 
it's mostly done arbitrarely and not consistantly :(

Also having 3 different Kirigami.Units.*Spacing values that are each only 2 
pixels appart doesn't sounds like a great idea as it's hard to see a 
difference between these values taken side by side.

I see three ways to move forward with this issue:

a) Remove smallSpacing and largeSpacing from Kirigami, and rename 
mediumSpacing to just spacing. This unified spacing value would be defined in 
qqc2-desktop-style to use whatever value is defined in the current QStyle.

a bis) Instead of creating only a generic "spacing" property, we create a 
"Kirigami.Units.margins" or "Kirigami.Units.paddings" property to use for 
paddings of QtQuick Controls and mapped to the Layout*Margin pixel metrics and 
a "Kirigami.Units.spacing" property mapped to the Layout*Spacing pixel 
metrics. For Breeze and Oxygen, both value would map to 6 pixels anyway, but 
it might make it easier to switch to other values in the future as well as 
make the usage of Units value more explit.

b) Use 4 pixels as standard spacing in our QtWidgets apps and add a "margins" 
and "largeMargin" helper methods in KWidgetsAddons or QStyle similar to this

QMargins QStyle::largeMargins() const
{
    return QMargins{
        pixelMetric(QStyle::PM_LayoutLeftMargin),
        pixelMetric(QStyle::PM_LayoutTopMargin),
        pixelMetric(QStyle::PM_LayoutRightMargin),
        pixelMetric(QStyle::PM_LayoutBottomMargin)
    }
}

QMargins QStyle::largeMargins() const
{
    return QMargins{
        pixelMetric(QStyle::PM_LayoutLeftMargin) * 2,
        pixelMetric(QStyle::PM_LayoutTopMargin) * 2,
        pixelMetric(QStyle::PM_LayoutRightMargin) * 2,
        pixelMetric(QStyle::PM_LayoutBottomMargin) * 2
    }
}

then we can remove mediumSpacing from Kirigami and ensure that in both our 
Kirigami and QtWidgets apps, we use small or large only no spacing at all. We 
should still try to define some guidelines when to use large or small spacing.

c) Do nothing and accept that spacing in Kirigami apps and QtWidgets are 
different. We might still want to define in our doc/hig the usecase for 
largeSpacing and smallSpacing

Personally I see advantages and disadvantages for all these solutions. I had a 
preference on b) but while writing this mail I'm slowly liking the a bis) idea 
more and more as we might not need a small spacing and large spacing and could 
get away with just one unified spacing.

Cheers,
Carl


[prev in list] [next in list] [prev in thread] [next in thread]