Code review comment for lp://staging/~intellectronica/launchpad/bugtask-ajax-assignee

Revision history for this message
Michael Nelson (michael.nelson) wrote :

r=noodles

* jtv has quit ("Leaving.")
<noodles775> intellectronica: yep, on it.
<intellectronica> awesome
<noodles775> wow... MP 3-0 :)
<noodles775> intellectronica: nice :) That works really well.
<noodles775> I'm a bit confused by the diff - there already was an InlineEditPickerWidget that you've removed... what's going on there?
* noodles775 looks to see how the editor widget is created now.
<noodles775> so the assignee_picker_widget property was never used?
<intellectronica> noodles775: there was some previous work done in this area by edwin, which worked in a completely different way from how the rest of the tasks table works. it was never used, and i changed or removed so as to make this work in a consistent way
<noodles775> Great.
<noodles775> intellectronica: not part of this code review, but looking at the page, there are a *lot* of edit icons staring back at me...
<noodles775> I wonder whether having so many edit icons in a small area is starting to make it hard to focus on the content... (12 are staring me down above the fold).
<intellectronica> noodles775: yes. in a future branch these will change to a slightly less offensive version martin has prepared. we're still not sure what the best solution is (if you remember for a while the icons were only shown on hover, but that was found to be not sufficiently discoverable)
<noodles775> Great!
<noodles775> intellectronica: while you're here, a quick question about the inline description editor... or should I wait for deryck?
<noodles775> Basically, I'm unsure why (1) the click/tab is a fixed width (ie try making the window smaller), and (2) why it needs a negative margin at the top.
<intellectronica> noodles775: try me (though chances i won't know the answer)
<intellectronica> noodles775: right, no idea. deryck might remember what's going on there since he last worked on this
<noodles775> (I was having to add special css exceptions on a page last night, but then stopped in case we can fix it generally with the inline editor).
<noodles775> OK.
<noodles775> adeuring: of course :)
<noodles775> intellectronica: great work! r=me

review: Approve (code)

« Back to merge proposal