Differential D23984 Diff 66205 sidebar/package/contents/ui/introPage.qml

Changeset View

Standalone View

View Options

sidebar/package/contents/ui/introPage.qml

Show All 19 Lines
20import QtQuick.Controls 2.5 as QQC2 20​import QtQuick.Controls 2.5 as QQC2
21import QtQuick.Layouts 1.1 21​import QtQuick.Layouts 1.1
22 22​
23import org.kde.kirigami 2.5 as Kirigami 23​import org.kde.kirigami 2.5 as Kirigami
24 24​
25Rectangle { 25​Rectangle {
26 id: root 26​ id: root
27 color: Kirigami.Theme.backgroundColor 27​ color: Kirigami.Theme.backgroundColor
28​ property int verticalMargin: 50
ngrahamUnsubmitted
Not Done

How about Kirigami.Units.gridUnit * 3 instead?

ngraham: How about `Kirigami.Units.gridUnit * 3` instead?
29​ property int iconsRectPadding: 20
30​ property int iconsRectRadius: 10
28 31​
ngrahamUnsubmitted
Not Done

Don't hardcode values here; instead use multiples of Kirigami.Units.gridUnit (or some other defined unit)

ngraham: Don't hardcode values here; instead use multiples of Kirigami.Units.gridUnit (or some other…
29 signal focusNextRequest() 32​ signal focusNextRequest()
30 signal focusPreviousRequest() 33​ signal focusPreviousRequest()
31 34​
32 function focusFirstChild() { 35​ function focusFirstChild() {
33 iconsRow.children[0].focus = true; 36​ iconsRow.children[0].focus = true;
34 } 37​ }
35 38​
36 function focusLastChild() { 39​ function focusLastChild() {
37 iconsRow.children[iconsRow.children.length-1].focus = true; 40​ iconsRow.children[iconsRow.children.length-1].focus = true;
38 } 41​ }
39 42​
40 ColumnLayout { 43​ RowLayout {
41 anchors { 44​ anchors {
42 bottom: separator.top 45​ bottom: parent.verticalCenter
43 bottomMargin: Kirigami.Units.largeSpacing 46​ bottomMargin: verticalMargin
44 horizontalCenter: parent.horizontalCenter 47​ horizontalCenter: parent.horizontalCenter
45 } 48​ }
49​ Layout.alignment: Qt.AlignHCenter
46 Kirigami.Icon { 50​ Kirigami.Icon {
47 Layout.alignment: Qt.AlignHCenter 51​ Layout.alignment: Qt.AlignHCenter
48 source: "preferences-system" 52​ source: "preferences-system"
49 width: Kirigami.Units.iconSizes.enormous 53​ width: Kirigami.Units.iconSizes.enormous
50 height: width 54​ height: width
51 opacity: 0.3 ​
52 } 55​ }
53 QQC2.Label { 56​ Item {
54 Layout.alignment: Qt.AlignHCenter 57​ // spacer item
ngrahamUnsubmitted
Not Done

Comment not really necessary; it's obvious that this is a spacer

ngraham: Comment not really necessary; it's obvious that this is a spacer
58​ width: Kirigami.Units.largeSpacing
59​ }
60​ ColumnLayout {
61​ Kirigami.Heading {
62​ text: i18n("KDE Plasma")
anthonyfieroniUnsubmitted
Done

Use i18n also only Plasma about me.

anthonyfieroni: Use i18n also only Plasma about me.
63​ font.pointSize: 24 // largest level is too small, so we use pointSize
anthonyfieroniUnsubmitted
Not Done

Don't hardcode font sizes use level instead
https://api.kde.org/frameworks-api/frameworks-apidocs/frameworks/kirigami/html/classorg_1_1kde_1_1kirigami_1_1Heading.html#ad694a087926d5fd83edc183b526db4a4

anthonyfieroni: Don't hardcode font sizes use level instead https://api.kde.org/frameworks-api/frameworks…
ngrahamUnsubmitted
Not Done

Just use level 1, it's big enough IMO (or make the icon next to the text smaller to compensate for the smaller text size)

ngraham: Just use level 1, it's big enough IMO (or make the icon next to the text smaller to compensate…
64​ }
65​ Kirigami.Heading {
55 text: i18n("System Settings") 66​ text: i18n("System Settings")
67​ level: 3
davidedmundsonUnsubmitted
Done

Hardcoding sizes is generally frowned upon.

Partly because things are user-configurable, partly because if every dev hardcodes their own sizes we'll have an inconsistent mess everywhere.

This patch has quite a few. Try and see what you can get rid of

davidedmundson: Hardcoding sizes is generally frowned upon. Partly because things are user-configurable…
martUnsubmitted
Done

if it's a kirigami.heading, it should use the automatic sizes from there.
All headings having the same size across everywhere is much more important than a perfect pixel alignment on a single place
(which won't really work anyways, because we are dealing with fonts and pojn ts there)

mart: if it's a kirigami.heading, it should use the automatic sizes from there. All headings having…
56 } 68​ }
57 } 69​ }
58 Kirigami.Separator { ​
59 id: separator ​
60 anchors.centerIn: parent ​
61 width: Math.round(parent.width * 0.8) ​
62 } 70​ }
63 ColumnLayout { 71​ Kirigami.Heading {
72​ id: frequentlyUsed
64 anchors { 73​ anchors {
65 top: separator.bottom 74​ top: parent.verticalCenter
66 topMargin: Kirigami.Units.largeSpacing 75​ topMargin: verticalMargin
67 horizontalCenter: parent.horizontalCenter 76​ horizontalCenter: parent.horizontalCenter
68 } 77​ }
69 width: Math.round(parent.width * 0.8) ​
70 Kirigami.Heading { ​
71 Layout.alignment: Qt.AlignHCenter 78​ Layout.alignment: Qt.AlignHCenter
72 level: 3 79​ level: 4
73 wrapMode: Text.NoWrap 80​ wrapMode: Text.NoWrap
74 text: i18n("Frequently used:") 81​ text: i18n("Frequently used:")
ngrahamUnsubmitted
Not Done

Why move this heading out of the ColumnLayout?

ngraham: Why move this heading out of the ColumnLayout?
75 } 82​ }
83​ Rectangle {
84​ anchors {
85​ top: frequentlyUsed.bottom
86​ topMargin: Kirigami.Units.largeSpacing
87​ horizontalCenter: parent.horizontalCenter
88​ }
89​ width: parent.width * 0.8
ngrahamUnsubmitted
Not Done

No need to add an id since nothing references it

ngraham: No need to add an id since nothing references it
ngrahamUnsubmitted
Not Done

Why change this? Rounding generally fixes the problem of blurriness with fractional scaling due to non-integer pixels.

ngraham: Why change this? Rounding generally fixes the problem of blurriness with fractional scaling due…
GB_2Unsubmitted
Not Done

Maybe change to "Frequently Used", so it's a header?

GB_2: Maybe change to "Frequently Used", so it's a header?
90​ height: childrenRect.height + 2 * iconsRectPadding
davidedmundsonUnsubmitted
Done

It's best to infer this from the child.

davidedmundson: It's best to infer this from the child.
91​ color: Qt.lighter(Kirigami.Theme.backgroundColor, 1.5) // brighter background using some heuristic
92​ radius: iconsRectRadius
93​ ColumnLayout {
94​ anchors {
martUnsubmitted
Done

add in a comment a link to https://en.wikipedia.org/wiki/Relative_luminance to explain the numbers

mart: add in a comment a link to https://en.wikipedia.org/wiki/Relative_luminance to explain the…
davidedmundsonUnsubmitted
Done

See https://doc.qt.io/qt-5/qml-qtqml-qt.html#darker-method

davidedmundson: See https://doc.qt.io/qt-5/qml-qtqml-qt.html#darker-method
martUnsubmitted
Done

right, since you are using brightness only to have a darker color, you should use Qt.darker()

mart: right, since you are using brightness only to have a darker color, you should use Qt.darker()
95​ top: parent.top
96​ topMargin: iconsRectPadding
97​ horizontalCenter: parent.horizontalCenter
98​ }
99​ width: parent.width - 2 * iconsRectPadding
76 RowLayout { 100​ RowLayout {
77 id: iconsRow 101​ id: iconsRow
78 Layout.fillWidth: true 102​ Layout.fillWidth: true
79 Layout.alignment: Qt.AlignHCenter 103​ Layout.alignment: Qt.AlignHCenter
80 spacing: Kirigami.Units.largeSpacing 104​ spacing: Kirigami.Units.largeSpacing
81 property int spaceForIcon: Math.max((iconsRow.parent.width - iconsRow.spacing * 4) / 5, Kirigami.Units.iconSizes.medium) 105​ property int spaceForIcon: Math.max((iconsRow.parent.width - iconsRow.spacing * 4) / 5, Kirigami.Units.iconSizes.medium)
82 property int iconSize: iconsRow.spaceForIcon >= Kirigami.Units.iconSizes.huge 106​ property int iconSize: iconsRow.spaceForIcon >= Kirigami.Units.iconSizes.huge
83 ? Kirigami.Units.iconSizes.huge 107​ ? Kirigami.Units.iconSizes.huge
Show All 9 Lines113 delegate: IntroIcon {
93 Layout.minimumWidth: iconsRow.spaceForIcon 117​ Layout.minimumWidth: iconsRow.spaceForIcon
94 Layout.maximumWidth: Layout.minimumWidth 118​ Layout.maximumWidth: Layout.minimumWidth
95 visible: (index + 1) * iconSize + index * iconsRow.spacing < iconsRow.parent.width 119​ visible: (index + 1) * iconSize + index * iconsRow.spacing < iconsRow.parent.width
96 } 120​ }
97 } 121​ }
98 } 122​ }
99 } 123​ }
100} 124​ }
125​
126​}