Comment 6 for bug 627686

Revision history for this message
Dylan McCall (dylanmccall) wrote :

Just played with it! Nice work.
(And extra points for figuring out my rather bizarre Javascript)

Some things, though…

First of all, I'm not keen on putting actions in brackets; that's one of the things that was rather vehemently pruned. I haven't thought about this much (I did at one point, then I forgot what I'd figured out), but perhaps make the whole label clickable and change it to "Show X hidden opportunities.” Having that label all lowercase may look better. If it does, edit ./media/css/style.css and move line 437 (“text-transform:lowercase;” in “.sourcepackage-details > .extra > .actions”) to the rule above it, for “.sourcepackage-details > .extra”

The new Show link needs Javascript to work. Just change its href to point to the same URL as permalink and it'll be fine for non-Javascript users. Well, another option is adding a superexpand=pkg_id GET parameter and using that; we'll need it if we want this state to persist when new results are loaded.
(Results.update_current_query will help you there, as used for packages with the expanded callback. I guess this will need a superexpanded callback, too. Eep! Wish I'd planned that bit better…).

Of course, you've probably noticed that the loading indicator doesn't clear. If you haven't: the loading indicator doesn't clear ;)
The reason is you're still calling the local “show” function in line 415, which causes an error so jquery gives up and the “complete” function is never called. No need to define the show function (unless we're doing fancy animations for showing the newly loaded results), so just remove that line and it works happily.

Your added code in harvest.js is using spaces to indent, but the rest of the file uses tabs to indent. (Yeah, I should have used spaces given the Python part of the project does, but there _is_ some bandwidth saved so I can claim that as my excuse).

Right now there's no obvious way to get back from showing all opportunities in a package.

I'm thinking it may be better if we stick these filtered out opportunities under a different header, rather than mixed in with the other ones. That would mean running the same SQL query we did before, but with a template that accesses the list of hidden opportunities as well. So, maybe the new stuff could appear _below_ the “Show hidden opportunities” button. (And that button morphs into something else). Needs some design changes, so maybe just something for the todo list, but I have a hunch the under the hood changes could make the rest of it a little more manageable…

Then again, I could be missing something really obvious, having not worked on this problem myself. I trust whatever you end up doing :)