Merge lp:~michael.nelson/launchpad/649559-ajax-comment-on-dsdiff-2 into lp:launchpad/db-devel
Status: | Merged | ||||
---|---|---|---|---|---|
Approved by: | Michael Nelson | ||||
Approved revision: | no longer in the source branch. | ||||
Merged at revision: | 9856 | ||||
Proposed branch: | lp:~michael.nelson/launchpad/649559-ajax-comment-on-dsdiff-2 | ||||
Merge into: | lp:launchpad/db-devel | ||||
Diff against target: |
337 lines (+190/-15) 7 files modified
lib/lp/registry/browser/configure.zcml (+3/-0) lib/lp/registry/browser/distroseriesdifference.py (+28/-2) lib/lp/registry/javascript/distroseriesdifferences_details.js (+136/-12) lib/lp/registry/model/distroseriesdifferencecomment.py (+3/-1) lib/lp/registry/templates/distroseriesdifference-listing-extra.pt (+4/-0) lib/lp/registry/templates/distroseriesdifferencecomment-fragment.pt (+2/-0) lib/lp/registry/windmill/tests/test_distroseriesdifference_expander.py (+14/-0) |
||||
To merge this branch: | bzr merge lp:~michael.nelson/launchpad/649559-ajax-comment-on-dsdiff-2 | ||||
Related bugs: |
|
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Abel Deuring (community) | code | Approve | |
Henning Eggers (community) | ui* | Approve | |
Curtis Hovey (community) | ui | Needs Fixing | |
Review via email: mp+37114@code.launchpad.net |
Commit message
Adds in-line commenting on DistroSeriesDif
Description of the change
Overview
========
This branch allows users to add comments to distro series differences. I'm initially just after a UI review, after which I'll add a windmill test and tidy-up the code.
See the LEP here:
https:/
and the specific mockup here:
You can view a 1min demo of the UI here:
http://
Details
=======
There were a few things I wasn't happy with code-wise in this branch:
1) Importing model code in a view simply to use order_by - is there a better way? (I tried a string instead ('IDistroSeries
2) Specifying the template manually for CommentXHTMLRep
To test:
========
bin/test -vvm test_distroseri
To demo locally:
================
Run http://
https:/
This looks really great! Thank you!
As discussed on IRC, though, I am really worried about the reverse ordering of the comments. Not that I don't like it, it is just inconsistent with other commenting we have in LP (bugs, mps, questions). You mentioned that you considered it more like a "wall" where people post notices. I see three possible solutions:
1. You simply adopt forward ordering for these comments and new comments are written at the bottom. I think it would be ok to only see latest 5 comments or so.
2. We consider adopting the reverse-ordered "wall" concept for all our commenting on Launchpad. That would have to be discussed on the mailing list but you already declined it yourself for MPs.
3. We make the "wall" concept visually different from the "comment" concept, so people are aware that they work differently. This should be more than just a different headline (although that would work as a temporary solution) and might take us back to your original design of the comments (I am sorry, I did not notice the reverse ordering then).
That said, I can only repeat that the page looks great and I just love to hear your voice! ;-)
Henning