Merge lp:~wallyworld/launchpad/link-bugs-in-merge-proposal into lp:launchpad

Proposed by Ian Booth
Status: Merged
Approved by: Ian Booth
Approved revision: no longer in the source branch.
Merged at revision: 11563
Proposed branch: lp:~wallyworld/launchpad/link-bugs-in-merge-proposal
Merge into: lp:launchpad
Diff against target: 94 lines (+29/-31)
2 files modified
lib/lp/code/stories/branches/xx-branchmergeproposals.txt (+0/-1)
lib/lp/code/templates/branchmergeproposal-index.pt (+29/-30)
To merge this branch: bzr merge lp:~wallyworld/launchpad/link-bugs-in-merge-proposal
Reviewer Review Type Date Requested Status
Tim Penhey (community) Approve
Ian Booth (community) ui Approve
Paul Hummer (community) Approve
Review via email: mp+34826@code.launchpad.net

Commit message

Move the linked bugs section on the branch merge proposal page to be just below the branch summary section, above the review comments.

Description of the change

Move the linked bugs section on the branch merge proposal page to be just below the branch summary section, above the review comments. I looked at putting the linked bugs in the unused space to the right of the branch summary details but this caused the branch and bug links in both sections to wrap. I added some horizontal white space just above the newly placed linked bugs section to make the page look better. The white space was added by wrapping the linked bugs div inside a <div class="first"> but I'm expecting there may be a better way - if so, I'll change it.

Tests:
    - no changes or new tests required.
    - ran the doc and windmill and unit tests for "branchmergeproposal" and
      "branchmergeproposals" (bin/test -t ...)

To post a comment you must log in.
Revision history for this message
Ian Booth (wallyworld) wrote :

Screenshot of new merge proposal screen:

https://devpad.canonical.com/~ianb/linkedbug.png

Revision history for this message
Ian Booth (wallyworld) wrote :

Oops wrong server - use this one:

https://people.canonical.com/~ianb/linkedbug.png

Revision history for this message
Ian Booth (wallyworld) wrote :

UI review suggested moving Linked Bugs section to below the Description, above the Comments

This has been done. I think it needs a horizontal separator line though, between the Linked Bugs and the "Add a Comment" button.

See https://devpad.canonical.com/~ianb/linkedbug2.png

Revision history for this message
Paul Hummer (rockstar) wrote :

Much better! Thanks for the changes!

review: Approve (ui)
Revision history for this message
Ian Booth (wallyworld) wrote :

This was a duplicate review request. Paul Hummer has approved the changes.

review: Approve (ui)
Revision history for this message
Tim Penhey (thumper) :
review: Approve
Revision history for this message
Tim Penhey (thumper) wrote :

The tal:condition="view/has_bug_or_spec" expression on the div before the title should be moved to the out-most div that has the class yui-g. That way we don't get extra whitespace if there are no bugs or blueprints.

Revision history for this message
Ian Booth (wallyworld) wrote :

As per discussion with Tim, the related bugs section has been made to line up with the description text and the section header has been removed.

See screenshot: http://people.canonical.com/~ianb/linkedbug3.png

Revision history for this message
Tim Penhey (thumper) wrote :

Picture?

Revision history for this message
Tim Penhey (thumper) wrote :

Sorry, didn't see the picture as the mail didn't thread properly. This looks good to me.

review: Approve
Revision history for this message
Tim Penhey (thumper) wrote :

Ian, did Paul get back to you about this?

Revision history for this message
Ian Booth (wallyworld) wrote :

Yes. I pinged him this morning and he said it was all ok. I've already
submitted the job to ec2

On 15/09/10 14:31, Tim Penhey wrote:
> Ian, did Paul get back to you about this?

Revision history for this message
Ian Booth (wallyworld) wrote :

Running through ec2 and a doc test failed (xx-branchmergeproposals.txt) due to the removal of the "Linked Bugs" header text from the page. Doc test now fixed (one line deletion).

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/code/stories/branches/xx-branchmergeproposals.txt'
2--- lib/lp/code/stories/branches/xx-branchmergeproposals.txt 2010-05-27 02:19:27 +0000
3+++ lib/lp/code/stories/branches/xx-branchmergeproposals.txt 2010-09-15 06:04:51 +0000
4@@ -543,7 +543,6 @@
5
6 >>> nopriv_browser.open(bmp_url)
7 >>> print_bugs_and_specs(nopriv_browser)
8- Linked bug reports and blueprints
9 Bug #...: Bug for linking Undecided New
10
11
12
13=== modified file 'lib/lp/code/templates/branchmergeproposal-index.pt'
14--- lib/lp/code/templates/branchmergeproposal-index.pt 2010-07-15 13:57:39 +0000
15+++ lib/lp/code/templates/branchmergeproposal-index.pt 2010-09-15 06:04:51 +0000
16@@ -46,6 +46,10 @@
17 #proposal-summary td {
18 padding-left: 0.5em;
19 }
20+ .related-bugs-list {
21+ padding-left: 20px;
22+ padding-bottom: 10px;
23+ }
24 /* A page-specific fix for inline text are editing to line up box. */
25 #edit-description .yui-ieditor-input { top: 0; }
26 #edit-commit_message .yui-ieditor-input { top: 0; }
27@@ -138,6 +142,18 @@
28 </tal:has-description>
29 </div>
30
31+ <div class="yui-g" tal:condition="view/has_bug_or_spec">
32+ <div id="related-bugs-and-blueprints" class="related-bugs-list"
33+ tal:define="branch context/source_branch">
34+ <div class="actions">
35+ <metal:bug-branch-links use-macro="branch/@@+macros/bug-branch-links"/>
36+ </div>
37+ <div class="actions">
38+ <metal:spec-branch-links use-macro="branch/@@+macros/spec-branch-links"/>
39+ </div>
40+ </div>
41+ </div>
42+
43 <div class="yui-g">
44 <tal:not-logged-in condition="not: view/user">
45 <div align="center" id="add-comment-login-first">
46@@ -176,36 +192,19 @@
47 </tal:logged-in>
48
49 <div class="yui-g">
50- <div class="yui-u first">
51- <div id="source-revisions"
52- tal:condition="not: context/queue_status/enumvalue:MERGED">
53-
54- <tal:history-available condition="context/source_branch/revision_count"
55- define="branch context/source_branch;
56- revisions view/unlanded_revisions">
57- <h2>Unmerged revisions</h2>
58- <metal:landing-target use-macro="branch/@@+macros/branch-revisions"/>
59- </tal:history-available>
60-
61- <tal:remote-branch condition="context/source_branch/branch_type/enumvalue:REMOTE">
62- <h2>Unmerged revisions</h2>
63- <p>Recent revisions are not available due to the source branch being remote.</p>
64- </tal:remote-branch>
65- </div>
66- </div>
67-
68- <div class="yui-u" tal:condition="view/has_bug_or_spec">
69- <div id="related-bugs-and-blueprints"
70- tal:define="branch context/source_branch">
71- <h2>Linked bug reports and blueprints</h2>
72- <div class="actions">
73-
74- <metal:bug-branch-links use-macro="branch/@@+macros/bug-branch-links"/>
75- </div>
76- <div class="actions">
77- <metal:spec-branch-links use-macro="branch/@@+macros/spec-branch-links"/>
78- </div>
79- </div>
80+ <div id="source-revisions"
81+ tal:condition="not: context/queue_status/enumvalue:MERGED">
82+
83+ <tal:history-available condition="context/source_branch/revision_count"
84+ define="branch context/source_branch;
85+ revisions view/unlanded_revisions">
86+ <h2>Unmerged revisions</h2>
87+ <metal:landing-target use-macro="branch/@@+macros/branch-revisions"/>
88+ </tal:history-available>
89+ <tal:remote-branch condition="context/source_branch/branch_type/enumvalue:REMOTE">
90+ <h2>Unmerged revisions</h2>
91+ <p>Recent revisions are not available due to the source branch being remote.</p>
92+ </tal:remote-branch>
93 </div>
94 </div>
95