Unity 8

Merge lp:~aacid/unity8/dashPreviewTableFixed1stColumnWidth into lp:unity8

  1. dashPreviewTableFixed1stColumnWidth
  2. Merge into trunk
Proposed by Albert Astals Cid
Status: Superseded
Proposed branch: lp:~aacid/unity8/dashPreviewTableFixed1stColumnWidth
Merge into: lp:unity8
Diff against target: 213 lines (+46/-7)
9 files modified
qml/Dash/Previews/PreviewAudioPlayback.qml (+2/-1)
qml/Dash/Previews/PreviewComment.qml (+2/-0)
qml/Dash/Previews/PreviewHeader.qml (+5/-3)
qml/Dash/Previews/PreviewIconActions.qml (+1/-0)
qml/Dash/Previews/PreviewRatingInput.qml (+1/-0)
qml/Dash/Previews/PreviewRatingSingleDisplay.qml (+3/-0)
qml/Dash/Previews/PreviewTable.qml (+7/-2)
qml/Dash/Previews/PreviewTextSummary.qml (+4/-1)
tests/qmltests/Dash/Previews/tst_PreviewTable.qml (+21/-0)
To merge this branch: bzr merge lp:~aacid/unity8/dashPreviewTableFixed1stColumnWidth
Reviewer Review Type Date Requested Status
Unity8 CI Bot continuous-integration Approve
Andrea Cimitan (community) Approve
Review via email: mp+307951@code.launchpad.net

This proposal has been superseded by a proposal from 2016-10-14.

Commit message

Fix first column of the preview table to be 25%

This way two consecutive tables have the second column start at the same X coordinate

Description of the change

 * Are there any related MPs required for this MP to build/function as expected?
No

 * Did you perform an exploratory manual test run of your code change and any related functionality?
Yes

 * If you changed the packaging (debian), did you subscribe the ubuntu-unity team to this MP?
N/A

 * If you changed the UI, has there been a design review?
Design driven change

To post a comment you must log in.
Revision history for this message
Unity8 CI Bot (unity8-ci-bot) wrote :

PASSED: Continuous integration, rev:2657
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/2340/
Executed test runs:
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build/3080
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/test-0-autopkgtest/label=amd64,release=vivid+overlay,testname=qmluitests.sh/1726
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/test-0-autopkgtest/label=amd64,release=xenial+overlay,testname=qmluitests.sh/1726
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/test-0-autopkgtest/label=amd64,release=yakkety,testname=qmluitests.sh/1726
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/3108
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/2965
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/2965/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/2965
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/2965/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=yakkety/2965
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=yakkety/2965/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/2965
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/2965/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/2965
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/2965/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=yakkety/2965
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=yakkety/2965/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/2965
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/2965/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/2965
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/2965/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=yakkety/2965
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=yakkety/2965/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/2340/rebuild

review: Approve (continuous-integration)
Revision history for this message
Andrea Cimitan (cimi) wrote :

http://i.imgur.com/6R0cu7M.png

wondering what to do in this case...

Revision history for this message
Albert Astals Cid (aacid) wrote :

Are you wondering about the "we could give more space to the first column" or "the first column wrapping looks ugly"?

Revision history for this message
Andrea Cimitan (cimi) wrote :

> Are you wondering about the "we could give more space to the first column" or
> "the first column wrapping looks ugly"?
Both issues... for the spacing between columns ok, the wrapping is tricky

lp:~aacid/unity8/dashPreviewTableFixed1stColumnWidth updated
2658. By Albert Astals Cid

workaround for bad row height on Qt 5.4 (on the phone)

Revision history for this message
Albert Astals Cid (aacid) wrote :

Added a workaround for the wrapping looking bad in Qt 5.4

About "knowing we could give more space to the 1st column", it's complicated, you'd have to recalculate everything for every other table on the preview column and check how long is the second column and etc, i understood this simple solution is what we wanted?

Revision history for this message
Andrea Cimitan (cimi) wrote :

 * Did you perform an exploratory manual test run of the code change and any related functionality?
y
 * Did CI run pass? If not, please explain why.
y

+1 for the test summary :)

review: Approve
Revision history for this message
Unity8 CI Bot (unity8-ci-bot) wrote :

PASSED: Continuous integration, rev:2658
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/2357/
Executed test runs:
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build/3109
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/test-0-autopkgtest/label=amd64,release=vivid+overlay,testname=qmluitests.sh/1754
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/test-0-autopkgtest/label=amd64,release=xenial+overlay,testname=qmluitests.sh/1754
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/test-0-autopkgtest/label=amd64,release=yakkety,testname=qmluitests.sh/1754
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/3137
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/2993
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/2993/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/2993
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/2993/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=yakkety/2993
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=yakkety/2993/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/2993
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/2993/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/2993
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/2993/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=yakkety/2993
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=yakkety/2993/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/2993
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/2993/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/2993
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/2993/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=yakkety/2993
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=yakkety/2993/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/2357/rebuild

review: Approve (continuous-integration)
lp:~aacid/unity8/dashPreviewTableFixed1stColumnWidth updated
2659. By Albert Astals Cid

bring lp:~unity-team/unity8/previews_fonts-update

2660. By Albert Astals Cid

Add a minimum height, design wants more breathing space in the table

2661. By Albert Astals Cid

less min height

2662. By Albert Astals Cid

remerge

2663. By Albert Astals Cid

More merging

2664. By Albert Astals Cid

use implicitHeight for this now since we force a minimumHeight the old test was failing

2665. By Albert Astals Cid

Try to make the vivid qt happy

2666. By Albert Astals Cid

Give the first column some more breathing space

Unmerged revisions

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'qml/Dash/Previews/PreviewAudioPlayback.qml'
2--- qml/Dash/Previews/PreviewAudioPlayback.qml 2016-02-16 03:24:40 +0000
3+++ qml/Dash/Previews/PreviewAudioPlayback.qml 2016-10-14 12:39:00 +0000
4@@ -105,7 +105,8 @@
5 anchors { top: parent.top; left: parent.left; right: parent.right }
6 opacity: 0.9
7 color: scopeStyle ? scopeStyle.foreground : theme.palette.normal.baseText
8- fontSize: "small"
9+ fontSize: "medium"
10+ font.weight: Font.Light
11 horizontalAlignment: Text.AlignLeft
12 text: modelData["title"]
13 elide: Text.ElideRight
14
15=== modified file 'qml/Dash/Previews/PreviewComment.qml'
16--- qml/Dash/Previews/PreviewComment.qml 2015-07-21 14:40:36 +0000
17+++ qml/Dash/Previews/PreviewComment.qml 2016-10-14 12:39:00 +0000
18@@ -70,6 +70,7 @@
19 visible: text !== ""
20 text: widgetData["subtitle"] || ""
21 fontSize: "xx-small"
22+ font.weight: Font.Light
23 maximumLineCount: 1
24 elide: Text.ElideRight
25 }
26@@ -77,6 +78,7 @@
27 width: parent.width
28 text: widgetData["comment"] || ""
29 fontSize: "small"
30+ font.weight: Font.Light
31 wrapMode: Text.Wrap
32 }
33 }
34
35=== modified file 'qml/Dash/Previews/PreviewHeader.qml'
36--- qml/Dash/Previews/PreviewHeader.qml 2016-03-10 22:39:24 +0000
37+++ qml/Dash/Previews/PreviewHeader.qml 2016-10-14 12:39:00 +0000
38@@ -60,7 +60,8 @@
39 anchors {
40 top: parent.top; left: parent.left; right: parent.right
41 margins: margins
42- leftMargin: spacing
43+ topMargin: 0
44+ leftMargin: 0
45 rightMargin: spacing
46 }
47
48@@ -83,6 +84,7 @@
49 sourceFillMode: UbuntuShape.PreserveAspectCrop
50 sourceHorizontalAlignment: UbuntuShape.AlignHCenter
51 sourceVerticalAlignment: UbuntuShape.AlignVCenter
52+ aspect: UbuntuShape.Flat
53 source: Image {
54 source: headerRoot.mascot
55 width: source ? mascotShapeLoader.width : 0
56@@ -113,8 +115,8 @@
57 rightMargin: iconLoader.width > 0 ? units.gu(0.5) : 0
58 }
59 elide: Text.ElideRight
60- font.weight: Font.Normal
61 fontSize: "large"
62+ font.weight: Font.Light
63 wrapMode: Text.Wrap
64 color: headerRoot.fontColor
65 text: headerRoot.title
66@@ -145,7 +147,7 @@
67 id: subtitleLabel
68 objectName: "subtitleLabel"
69 elide: Text.ElideRight
70- fontSize: "small"
71+ fontSize: "x-small"
72 font.weight: Font.Light
73 color: headerRoot.fontColor
74 text: headerRoot.subtitle
75
76=== modified file 'qml/Dash/Previews/PreviewIconActions.qml'
77--- qml/Dash/Previews/PreviewIconActions.qml 2016-04-01 15:34:03 +0000
78+++ qml/Dash/Previews/PreviewIconActions.qml 2016-10-14 12:39:00 +0000
79@@ -71,6 +71,7 @@
80 anchors.left: icon.right
81 anchors.leftMargin: visible ? units.gu(0.5) : 0
82 text: modelData.label
83+ font.weight: Font.Light
84 visible: text !== ""
85 }
86
87
88=== modified file 'qml/Dash/Previews/PreviewRatingInput.qml'
89--- qml/Dash/Previews/PreviewRatingInput.qml 2016-08-18 15:02:04 +0000
90+++ qml/Dash/Previews/PreviewRatingInput.qml 2016-10-14 12:39:00 +0000
91@@ -103,6 +103,7 @@
92 objectName: "ratingLabel"
93 anchors { left: parent.left; right: parent.right; }
94 fontSize: "large"
95+ font.weight: Font.Light
96 color: root.scopeStyle ? root.scopeStyle.foreground : theme.palette.normal.baseText
97 opacity: .8
98 text: widgetData["rating-label"] || i18n.tr("Rate this")
99
100=== modified file 'qml/Dash/Previews/PreviewRatingSingleDisplay.qml'
101--- qml/Dash/Previews/PreviewRatingSingleDisplay.qml 2015-06-18 10:07:37 +0000
102+++ qml/Dash/Previews/PreviewRatingSingleDisplay.qml 2016-10-14 12:39:00 +0000
103@@ -47,6 +47,7 @@
104 objectName: "authorLabel"
105 anchors { left: parent.left; right: parent.right }
106 opacity: .8
107+ font.size: "small"
108 visible: text !== ""
109 wrapMode: Text.Wrap
110 }
111@@ -57,6 +58,8 @@
112 anchors { left: parent.left; right: parent.right }
113 color: authorLabel.color
114 opacity: .8
115+ font.size: "x-small"
116+ font.weight: Font.Light
117 visible: text !== ""
118 wrapMode: Text.Wrap
119 }
120
121=== modified file 'qml/Dash/Previews/PreviewTable.qml'
122--- qml/Dash/Previews/PreviewTable.qml 2015-07-15 15:13:18 +0000
123+++ qml/Dash/Previews/PreviewTable.qml 2016-10-14 12:39:00 +0000
124@@ -48,6 +48,7 @@
125 }
126 height: visible ? implicitHeight : 0
127 fontSize: "large"
128+ font.weight: Font.Light
129 color: root.scopeStyle ? root.scopeStyle.foreground : theme.palette.normal.baseText
130 visible: text !== ""
131 opacity: .8
132@@ -73,10 +74,14 @@
133 text: perRowRepeater.model[index]
134 visible: root.expanded || rowIndex < maximumCollapsedRowCount
135 color: root.scopeStyle ? root.scopeStyle.foreground : theme.palette.normal.baseText
136- font.bold: index == 0
137+ font.weight: index == 0 ? Font.Normal : Font.Light
138 wrapMode: Text.Wrap
139 Layout.alignment: Qt.AlignTop
140- Layout.maximumWidth: column.width - x
141+ Layout.maximumWidth: index == 0 ? column.width / 4 : column.width - x
142+ Layout.minimumWidth: index == 0 ? column.width / 4 : -1
143+ height: -1 // FIXME Qt 5.4 needs this otherwise wrapped columns
144+ // get the height wrong and the next row looks weird
145+ // remove once we stop supporting Qt 5.4 (if 5.5 doesn't need it)
146 }
147 }
148 }
149
150=== modified file 'qml/Dash/Previews/PreviewTextSummary.qml'
151--- qml/Dash/Previews/PreviewTextSummary.qml 2015-07-15 15:07:19 +0000
152+++ qml/Dash/Previews/PreviewTextSummary.qml 2016-10-14 12:39:00 +0000
153@@ -39,6 +39,7 @@
154 }
155 height: visible ? implicitHeight : 0
156 fontSize: "large"
157+ font.weight: Font.Light
158 color: root.scopeStyle ? root.scopeStyle.foreground : theme.palette.normal.baseText
159 visible: text !== ""
160 opacity: .8
161@@ -56,10 +57,12 @@
162 left: parent.left
163 right: parent.right
164 top: titleLabel.visible ? titleLabel.bottom : parent.top
165+ topMargin: units.gu(1.5)
166 }
167 height: (lineCount <= maximumCollapsedLineCount || root.expanded) ? contentHeight : contentHeight / lineCount * maximumCollapsedLineCount
168 clip: true
169- fontSize: "small"
170+ fontSize: "medium"
171+ font.weight: Font.Light
172 lineHeight: 1.2
173 color: root.scopeStyle ? root.scopeStyle.foreground : theme.palette.normal.baseText
174 opacity: .8
175
176=== modified file 'tests/qmltests/Dash/Previews/tst_PreviewTable.qml'
177--- tests/qmltests/Dash/Previews/tst_PreviewTable.qml 2015-07-15 15:07:19 +0000
178+++ tests/qmltests/Dash/Previews/tst_PreviewTable.qml 2016-10-14 12:39:00 +0000
179@@ -31,6 +31,11 @@
180 "values": [ [ "Long Label 1", "Long Value 1 Long Value 2 Long Value 3 Long Value 4 Long Value 5 Long Value 6 Long Value 2 Long Value 2 Long Value 2 Long Value 2"], [ "Label 2", "Long Value 2 Long Value 2 Long Value 2 Long Value 2 Long Value 2 Long Value 2 Long Value 2 Long Value 2 Long Value 2 Long Value 2"], [ "Label 3", "Value 3"], [ "Label 4", "Value 4"], [ "Label 5", "Value 5"] ]
181 }
182
183+ property var widgetDataComplete2: {
184+ "title": "Short Title here",
185+ "values": [ [ "Publisher/Creator", "Unity8 team" ], [ "Label A", "Long Value 1 Long Value 2 Long Value 3 Long Value 4 Long Value 5 Long Value 6 Long Value 2 Long Value 2 Long Value 2 Long Value 2"], [ "Author", "Best 3v3r"], [ "Summary", "If i knew how to write summaries i'd be working somewhere else"] ]
186+ }
187+
188 property var widgetDataNoTitle: {
189 "values": [ [ "Long Label 1", "Long Value 1 Long Value 2 Long Value 3 Long Value 4 Long Value 5 Long Value 6 Long Value 2 Long Value 2 Long Value 2 Long Value 2"], [ "Label 2", "Long Value 2 Long Value 2 Long Value 2 Long Value 2 Long Value 2 Long Value 2 Long Value 2 Long Value 2 Long Value 2 Long Value 2"], [ "Label 3", "Value 3"], [ "Label 4", "Value 4"], [ "Label 5", "Value 5"] ]
190 }
191@@ -51,6 +56,22 @@
192 }
193 }
194
195+ PreviewWidgetFactory {
196+ id: previewTable2
197+ anchors { left: parent.left; right: parent.right; top: previewTable.bottom; topMargin: units.gu(4) }
198+ widgetType: "table"
199+
200+ Rectangle {
201+ color: "red"
202+ anchors.fill: parent
203+ opacity: 0.5
204+ }
205+
206+ Component.onCompleted: {
207+ previewTable2.widgetData = widgetDataComplete2
208+ }
209+ }
210+
211 UT.UnityTestCase {
212 name: "PreviewTableTest"
213 when: windowShown

Subscribers

People subscribed via source and target branches