Spacing in our apps

[Nate Graham]

This is an important topic, so I appreciate you bringing it up, Carl. 
It's been on my mind recently as well.

For some background, the reason why we have multiple spacing values in 
Kirigami and Plasma (e.g. SmallSpacing, LargeSpacing, GridUnit, etc) is 
because in the past we didn't want developers multiplying standard 
spacing values by "random numbers." The idea was that developers would 
choose the perfect spacing value for each situation and use it directly, 
without having to multiply it by anything.

Unfortunately, we failed to make it obvious what situation merited what 
spacing value. So in practice individual developers ended up using 
whichever spacing values felt best to them, and as we know, opinions 
differ on the topic of padding vs density in UI layouts. The result is 
inconsistent spacings in many places.

In addition, people end up multiplying the standard spacings by random 
numbers anyway, because most of them are quite small and sometimes you 
do need big spacing.

Finally, until Noah added MediumSpacing fairly recently, none of the 
standard spacing values matched the default spacing value in Breeze of 
6px. Even then the spacings may not remain a perfect match because the 
Kirigami/Plasma spacing values adjust with the font size, while the 
Breeze one does not.

I think we can unfortunately conclude that this design system was not a 
success, and in fact worsened the consistency of our spacings in 
QtQuick-based software compared to QtWidgets-based software while 
confusing developers.

As such, I support *proposal A*, which I feel is the simplest and most 
comprehensible one.

I could accept "A bis", but I think having two standard values that get 
defined to the same underlying value in Breeze is recipe for continuing 
the confusion, just in a different form. What's the difference between a 
spacing and a margin, anyway? We might want to move away from continuing 
this distinction in our QStyle too, seeing as we define both values to 
the same thing.


Nate




On 12/17/23 05:21, Carl Schwan wrote:
> 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
> 
>