should be possible to display non listed opportunities

Bug #627686 reported by Sebastien Bacher
10
This bug affects 2 people
Affects Status Importance Assigned to Milestone
harvest
In Progress
Low
Nigel Jones

Bug Description

Browsing a specific set filtered on selected options you get a list with all the sources in the set and the matching opportunities then a label counting the non listed ones. It would be nice to be able to click on that label to display those for a specific source.

Example reviewing build issues on the ubuntu-desktop set, having:

"<source>

 * #bugnumber, opportunity

some opportunities are not listed, click to display those"

would be nice

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

This was in the earlier design notes. Indeed, would be nice :)

Right now you can do that by clicking the Permalink button on the bottom right, though it's a little unintuitive.

This is similar to another thing that needs to happen: hiding opportunities that have been marked invalid / applied. Right now they are just crossed out, but they should actually be collapsed the same way as opportunities that are filtered out normally. Ideally the solution to this could be applied to that, too.

Revision history for this message
Nigel Jones (dev-nigelj) wrote :

Dylan,

I've been pondering this, what I'm thinking of (and willing to investigate/implement) is:

Use jQuery UI dialogs (we are already using jQuery) to show an inline pop-up (similar to Launchpad), of the same content as Permalink shows, the only differences would be: The headers/footers of the generated pages would be removed (i.e. it would just display the essentials).

My thoughts for doing it this way is that we aren't then over complicating SQL queries and slowing down the main listings, we are providing the content in one window & on demand.

Text browsers upon clicking on the 'Show' link would be redirected to the full Permalink page.

I'll try and come up w/ a concept branch.

Revision history for this message
Nigel Jones (dev-nigelj) wrote :

Actually, on second thoughts, I'm going to try and do this similar to how the main opportunity lists are done in the first place (i.e. w/ xhr).

Revision history for this message
Dylan McCall (dylanmccall) wrote : Re: [Harvest-dev] [Bug 627686] Re: should be possible to display non listed opportunities

That's a good point and I agree with you about keeping it simple, but why
not load that new content inside the existing package container?

I have a bit of an issue with a dialogue box type thing because, while
pretty, those are completely modal. Everything else in our UI is (or at
least should be) nonmodal. Not that there can't be some exceptions, of
course, like for things that really just need to be modal. (Forms spring to
mind).

Thank you for looking into this!

On 2010-09-03 5:55 AM, "Nigel Jones" <email address hidden> wrote:
Dylan,

I've been pondering this, what I'm thinking of (and willing to
investigate/implement) is:

Use jQuery UI dialogs (we are already using jQuery) to show an inline
pop-up (similar to Launchpad), of the same content as Permalink shows,
the only differences would be: The headers/footers of the generated
pages would be removed (i.e. it would just display the essentials).

My thoughts for doing it this way is that we aren't then over
complicating SQL queries and slowing down the main listings, we are
providing the content in one window & on demand.

Text browsers upon clicking on the 'Show' link would be redirected to
the full Permalink page.

I'll try and come up w/ a concept branch.

--
should be possible to display non listed opportunities
https://bugs.launchpad.net/bugs/627686
Y...

Nigel Jones (dev-nigelj)
Changed in harvest:
status: New → In Progress
assignee: nobody → Nigel Jones (dev-nigelj)
Revision history for this message
Nigel Jones (dev-nigelj) wrote :

Dylan, yes, thats what I was thinking and managed to implement.

At the moment it is a little crude, I want to choose a better way to tell Django etc that we are requesting all results, but this is certainly the fastest & easiest way of displaying all opportunities.

The other issue is that it doesn't stick (when someone changes the filter options), but it is fixable and I'll update that before proposing for merge.

Whats your thoughts?

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 :)

Revision history for this message
Nigel Jones (dev-nigelj) wrote :

Documenting discussion from IRC:

We decided that he best practice was to:

* Allow the undoing of the 'superexpand' - added a link (where X Opportunities Hidden would be seen) reading "Return to original results" - this copies the original results to a variable to be restored w/o additional queries
* When collapsing a package, also under the show all opportunities
* Change the X Opps Hidden text to: Show x hidden opportunities (Show hidden opportunity for 1 hidden)

I have also, as suggested:
* Fixed the non-javascript links (was already intended) & made the change to CSS to reformat/lowercase text
* Fixed the animations (i.e. turn off when loading is finished)

I think this is the best all-round solution for the issue.

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

One more suggestion, this time in templates/opportunities/include/package_details.html

Right now the plural string for the link that superexpands a package is:
“Show 32 hidden opportunities”
And the singular string is:
“Show hidden opportunity”

When I read that the first time the non-plural string kind of surprised me because it looked like something different and it wasn't immediately clear that a hidden opportunity was in a variable set. I think a reason there is the number is the most prominent part of the plural label.
Naturally, the fix is simple enough if you want: change that to "Show {{ counter }} hidden opportunity"
That would keep it nice and consistent.

Revision history for this message
Nigel Jones (dev-nigelj) wrote :

I'm not sure I agree, originally we were just telling people there were X opportunities hidden,

Now we are saying to them that they can see the hidden opportunities by clicking here.

Personally I'd sooner add 'the' to the string so it read: Show the hidden opportunity, and Show the X hidden opportunities. I'm no grammar expert, but Show 1 hidden opportunity didn't sound right.

The other option is:

"No hidden opportunities"
"Show hidden opportunit(y|ies) ({{counter}})"

Or

"Currently displaying x opportunities out of y for this package" OR "Displaying x of y opportunit(y|ies) for this package- Text/statement
"Show the filtered opportunity"/"Show the X filtered opportunities" OR "Show all opportunities for this package" - Link to expand

They all say the same thing, and to be honest I'm no linguist or translation expert, but I'm sure there is one option that is better suited to common translation than the others.

Changed in harvest:
importance: Undecided → Low
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.