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
