Merge lp:~deadlight/ubuntu-cdimage/ubuntu-cdimage into lp:ubuntu-cdimage
- ubuntu-cdimage
- Merge into mainline
Status: | Merged |
---|---|
Merged at revision: | 1763 |
Proposed branch: | lp:~deadlight/ubuntu-cdimage/ubuntu-cdimage |
Merge into: | lp:ubuntu-cdimage |
Diff against target: |
117 lines (+32/-20) 1 file modified
lib/cdimage/tree.py (+32/-20) |
To merge this branch: | bzr merge lp:~deadlight/ubuntu-cdimage/ubuntu-cdimage |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Dimitri John Ledkov (community) | Approve | ||
Steve Langasek | Approve | ||
Review via email:
|
Commit message
Fix for the nesting of <div>s in the heading of listing pages.
Description of the change
Fix for the nesting of <div>s in the heading of listing pages.
- 1756. By Karl Williams <email address hidden>
-
Remove comment and reformat long line
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Karl Williams (deadlight) wrote : | # |
Fixed, sorry about that
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Steve Langasek (vorlon) wrote : | # |
I don't think the fix for the nesting worked as intended: http://
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Steve Langasek (vorlon) wrote : | # |
I don't think the fix for the nesting worked as intended: http://
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Karl Williams (deadlight) wrote : | # |
Hmm, I moved something to the wrong level of the loop.
What parameters are you running the generator with?
On Wed, 14 Nov 2018, 17:26 Steve Langasek, <email address hidden>
wrote:
> I don't think the fix for the nesting worked as intended:
> http://
> --
>
> https:/
> You are the owner of lp:~deadlight/ubuntu-cdimage/ubuntu-cdimage.
>
> Launchpad-
> Launchpad-
> Launchpad-
> Launchpad-Branch: ~deadlight/
> Launchpad-Project: ubuntu-cdimage
>
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Steve Langasek (vorlon) wrote : | # |
On Wed, Nov 14, 2018 at 05:39:24PM -0000, Karl Williams wrote:
> What parameters are you running the generator with?
This was:
DIST=bionic make-web-indices .bionic-vanilla/ ubuntu-18.04.1 release
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Karl Williams (deadlight) wrote : | # |
Great, thanks. I'll create a similar directory and use those params to test.
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Karl Williams (deadlight) wrote : | # |
Ok, I think that *THIS* fixes the HTML nesting.
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Steve Langasek (vorlon) wrote : | # |
Thanks, http://
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Karl Williams (deadlight) wrote : | # |
Thanks Steve...
Multiple options for images needs something.
- 1759. By Karl Williams <email address hidden>
-
Fix indent dor closing </div> and remove unneeded content
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Karl Williams (deadlight) wrote : | # |
I've tested for a single-image build and one with multiple images per release. It all looks good to me.
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Steve Langasek (vorlon) wrote : | # |
Yes, http://
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Karl Williams (deadlight) wrote : | # |
Dimitri: How's this looking? Can we consider it fixed?
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Karl Waghorn-Moyce (karlwaghorn-moyce) wrote : | # |
Dimitri: Did you get the chance to check Karl's updates?
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Dimitri John Ledkov (xnox) wrote : | # |
This is ok.
There is a further improvement that I would want to see:
* icons should be vertically centered on the left
* Name column must not be wrapped
* and on mobile, table should be `pannable` (such that it takes more than the width of the page, which one can smudge left/right with thumb - no idea how that is called properly) or, like can we force a vertical layout of rows on mobiles? https:/
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Karl Williams (deadlight) wrote : | # |
Dimitri: Thanks for the feedback.
1. I'll override apache's default top alignment here
2. I'll make sure that the name column gets enough space
3. I can, initially, give the table a min-width to enable the side-scrolling behaviour that you described. A vertical layout might work but is a little more complex, especially when we don't have complete control over how the HTML is generated.
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Karl Williams (deadlight) wrote : | # |
Dimitri: I've pushed some commits to this branch to make the above improvements.
Preview Diff
1 | === modified file 'lib/cdimage/tree.py' | |||
2 | --- lib/cdimage/tree.py 2018-11-12 19:19:26 +0000 | |||
3 | +++ lib/cdimage/tree.py 2018-11-28 13:21:16 +0000 | |||
4 | @@ -1195,7 +1195,12 @@ | |||
5 | 1195 | for css in self.cssincludes(): | 1195 | for css in self.cssincludes(): |
6 | 1196 | print(" @import url(%s);" % css, file=header) | 1196 | print(" @import url(%s);" % css, file=header) |
7 | 1197 | print(dedent("""\ | 1197 | print(dedent("""\ |
8 | 1198 | .p-table-wrapper { | ||
9 | 1199 | overflow-x: scroll; | ||
10 | 1200 | } | ||
11 | 1201 | |||
12 | 1198 | table { | 1202 | table { |
13 | 1203 | min-width: 984px; | ||
14 | 1199 | width: 100%; | 1204 | width: 100%; |
15 | 1200 | } | 1205 | } |
16 | 1201 | 1206 | ||
17 | @@ -1206,8 +1211,28 @@ | |||
18 | 1206 | 1211 | ||
19 | 1207 | th:first-child, | 1212 | th:first-child, |
20 | 1208 | td:first-child { | 1213 | td:first-child { |
21 | 1214 | vertical-align: inherit; | ||
22 | 1209 | width: 5%; | 1215 | width: 5%; |
23 | 1210 | } | 1216 | } |
24 | 1217 | |||
25 | 1218 | th:nth-of-type(2), | ||
26 | 1219 | td:nth-of-type(2) { | ||
27 | 1220 | width: 20em; | ||
28 | 1221 | } | ||
29 | 1222 | |||
30 | 1223 | th:nth-of-type(3), | ||
31 | 1224 | td:nth-of-type(3) { | ||
32 | 1225 | width: 12em; | ||
33 | 1226 | } | ||
34 | 1227 | |||
35 | 1228 | th:nth-of-type(4), | ||
36 | 1229 | td:nth-of-type(4) { | ||
37 | 1230 | width: 6em; | ||
38 | 1231 | } | ||
39 | 1232 | |||
40 | 1233 | th:nth-of-type(5), | ||
41 | 1234 | td:nth-of-type(5) { | ||
42 | 1235 | } | ||
43 | 1211 | </style> | 1236 | </style> |
44 | 1212 | """), file=header) | 1237 | """), file=header) |
45 | 1213 | if self.project == "kubuntu": | 1238 | if self.project == "kubuntu": |
46 | @@ -1326,8 +1351,6 @@ | |||
47 | 1326 | "https://help.ubuntu.com/community/BitTorrent", "BitTorrent") | 1351 | "https://help.ubuntu.com/community/BitTorrent", "BitTorrent") |
48 | 1327 | 1352 | ||
49 | 1328 | for prefix in prefixes: | 1353 | for prefix in prefixes: |
50 | 1329 | print('<div class="row p-divider"><div class="p-card">', | ||
51 | 1330 | file=header) | ||
52 | 1331 | for publish_type in all_publish_types: | 1354 | for publish_type in all_publish_types: |
53 | 1332 | if not self.find_images(directory, prefix, publish_type): | 1355 | if not self.find_images(directory, prefix, publish_type): |
54 | 1333 | continue | 1356 | continue |
55 | @@ -1365,6 +1388,8 @@ | |||
56 | 1365 | if not paths: | 1388 | if not paths: |
57 | 1366 | continue | 1389 | continue |
58 | 1367 | 1390 | ||
59 | 1391 | print('<div class="row p-divider"><div class="p-card">', | ||
60 | 1392 | file=header) | ||
61 | 1368 | cdtypestr = self.cdtypestr(publish_type, image_format) | 1393 | cdtypestr = self.cdtypestr(publish_type, image_format) |
62 | 1369 | print('<div class="col-6 p-divider__block">', | 1394 | print('<div class="col-6 p-divider__block">', |
63 | 1370 | file=header) | 1395 | file=header) |
64 | @@ -1376,20 +1401,6 @@ | |||
65 | 1376 | print(tag, file=header) | 1401 | print(tag, file=header) |
66 | 1377 | print(file=header) | 1402 | print(file=header) |
67 | 1378 | 1403 | ||
68 | 1379 | if len(paths) == 1: | ||
69 | 1380 | print( | ||
70 | 1381 | "<p>There is one image available:</p>", | ||
71 | 1382 | file=header) | ||
72 | 1383 | elif publish_type == "src": | ||
73 | 1384 | print( | ||
74 | 1385 | "<p>There are %s images available:</p>" % | ||
75 | 1386 | self.numbers[len(paths)], file=header) | ||
76 | 1387 | else: | ||
77 | 1388 | print( | ||
78 | 1389 | "<p>There are %s images available, each for a " | ||
79 | 1390 | "different type of computer:</p>" % | ||
80 | 1391 | self.numbers[len(paths)], file=header) | ||
81 | 1392 | |||
82 | 1393 | print(file=header) | 1404 | print(file=header) |
83 | 1394 | 1405 | ||
84 | 1395 | print('</div>', file=header) | 1406 | print('</div>', file=header) |
85 | @@ -1424,7 +1435,6 @@ | |||
86 | 1424 | self.titlecase(cdtypestr), archstr) | 1435 | self.titlecase(cdtypestr), archstr) |
87 | 1425 | archdesc = self.archdesc(arch, publish_type) | 1436 | archdesc = self.archdesc(arch, publish_type) |
88 | 1426 | 1437 | ||
89 | 1427 | print("<div>", file=header) | ||
90 | 1428 | if os.path.exists(path): | 1438 | if os.path.exists(path): |
91 | 1429 | print( | 1439 | print( |
92 | 1430 | "<a href=\"%s\">%s</a>" % | 1440 | "<a href=\"%s\">%s</a>" % |
93 | @@ -1492,9 +1502,9 @@ | |||
94 | 1492 | htaccessimagestr, extstr, | 1502 | htaccessimagestr, extstr, |
95 | 1493 | os.path.basename(extpath)), | 1503 | os.path.basename(extpath)), |
96 | 1494 | file=htaccess) | 1504 | file=htaccess) |
97 | 1495 | print("</div>", file=header) | ||
98 | 1496 | print('</div>', file=header) | 1505 | print('</div>', file=header) |
100 | 1497 | print('</div></div>', file=header) | 1506 | print('</div></div>', file=header) |
101 | 1507 | |||
102 | 1498 | published_ec2_path = os.path.join( | 1508 | published_ec2_path = os.path.join( |
103 | 1499 | directory, "published-ec2-%s.txt" % status) | 1509 | directory, "published-ec2-%s.txt" % status) |
104 | 1500 | if os.path.exists(published_ec2_path): | 1510 | if os.path.exists(published_ec2_path): |
105 | @@ -1616,9 +1626,11 @@ | |||
106 | 1616 | print(tag, file=header) | 1626 | print(tag, file=header) |
107 | 1617 | print(file=header) | 1627 | print(file=header) |
108 | 1618 | 1628 | ||
109 | 1629 | print("<div class='p-table-wrapper'>", file=header) | ||
110 | 1630 | |||
111 | 1619 | print( | 1631 | print( |
112 | 1620 | dedent("""\ | 1632 | dedent("""\ |
114 | 1621 | </div></div></div> | 1633 | </div></div></div></div> |
115 | 1622 | <footer class="p-footer"> | 1634 | <footer class="p-footer"> |
116 | 1623 | <div class="row"> | 1635 | <div class="row"> |
117 | 1624 | <p><small>© 2018 Canonical Ltd. Ubuntu and Canonical | 1636 | <p><small>© 2018 Canonical Ltd. Ubuntu and Canonical |
Could you please run ./run-tests from the tree before pushing?
....... ....... ....... ....... ....... ....... ....... ....... ....... ....... ....... ....... ....... ....... /lib/cdimage/ tree.py: 1340:25: E129 visually indented line with same indent as next logical line tree.py: 1340:60: E202 whitespace before ')' tree.py: 1341:80: E501 line too long (93 > 79 characters) tree.py: 1495:59: E261 at least two spaces before inline comment tree.py: 1495:60: E262 inline comment should start with '# ' ....... ....... ....... ....... ....... ....... ....... ....... ....... ....... ....... ....... ....... ....... ....... ....... ....... ....... ....... ....... ....... ....... ....... ....... ....... ....... ....... ....... ....... ....... ....... ....... ....... ....... ....... ....... ....... ....... ....... ....... ....... ....... ....... ....... ....... ....... ....... ....... ....... ....... ....... ....... ....... ....... ....... ....... ....... ....... ....... ....... ....... ....... ....... ....... ....... ....... ....... ...... ======= ======= ======= ======= ======= ======= ======= ======= ======= tests.test_ static. TestStatic) ------- ------- ------- ------- ------- ------- ------- ------- ------- vorlon/ devel/canonical /cdimage/ lib/cdimage/ tests/test_ static. py", line 63, in test_pep8_clean assertEqual( 0, len(output))
./lib/cdimage/
./lib/cdimage/
./lib/cdimage/
./lib/cdimage/
F......
=======
FAIL: test_pep8_clean (cdimage.
-------
Traceback (most recent call last):
File "/home/
self.
AssertionError: 0 != 5
------- ------- ------- ------- ------- ------- ------- ------- ------- -------
Ran 580 tests in 9.908s
FAILED (failures=1)