Single click show/hide track list.

[asermax]

The title says it all. Quoting @jrbastien:

I just upgraded my Itunes version on Windows and see what you mean. The feature that I like the most is in the album view, on a single left click, you get the track list. Then if you click again on the album it hides the list.

In RB, I always do this: Click on a cover, then expand the track list at the bottom then I hide it again to continue browsing. That would be really nice, if you could implement this feature on a single click like in itunes. No need for the color morphic effects but just the track list as it is at the bottom.

[fossfreedom]

As far as I understand this on itunes - it works like this.

1. on first click of an album, the window splits horizontally and the track view appear.
2. on second click of the same album the track view animates away revealing the album view
3. click on a different album the track view remains but updates with details of the album
4. on a ctrl + click on a second album (i.e. now have two albums selected), the track view disappears - I dont see an option to see the tracks of multiple selected albums
5. on a ctrl + click of an album (so just one album is selected) - the track view does not appear
6. if clicking an album that would normally be hidden by the track view, the album view is scrolled until you can see both the track view and selected album.

[asermax]

Hmm I think we should scatch 4 and 5 and make it behave like 3. Also, there should be some kind of setting to define the default behavior, either to automatically show the bottom pane or not. And some kind of manual action to show/hide the bottom pane should would still be needed. I would like to scratch the expander and add a keybinding, but some people are not very fond of keybindings, so maybe keeping the expander and adding the keybinding would be the best way to go.

We should take some time to check out the different combinations of actions to make sure that the bottom pane behaves correctly in all of them, for example:

1. The default behavior is to automatically show the bottom pane.
2. The user selects a couple of albums.
3. The user manually hides the bottom pane.
4. The user selects another album, along the ones that were already selected.

In this use case, I think the correct behavior would be not to show the bottom pane again.

[fossfreedom]

ok - I've made some minor amendments for this on the 'click' branch.

Let me know your thoughts. I havent added a keybinding - I presume some sort of CTRL+ALT+key is to toggle the expander that you would normally do via the mouse?

[jrbastien]

I have been following your progresses on this one. Although, it is in line with Asermax recommendation, I think the itunes behavior is more convenient:

• First click, the album gets the focus
• Second click, the track list is expanded
• Third click, the track list is hidden again

Yes, I can manually hide the track list but it does not mean that I don't want to see it again if I click on another album. This is why not expanding the list immediately is important. This way, you can decide to expand it or not by clicking on the album that has the focus or just select another album.

[asermax]

@fossfreedom it seems to be working good so far. Keep on it :3

@jrbastien the second click to show the track list may be an issue, since the double click gesture is already used to play an album. Taking into acount that you may want to see the tracks right away, the secound click could be really close to the first and end up as a double click.

I think you have a point when you say that even if you close the track list, you may want it to show again when you select another album (when you close the track list to look for another album), but the other use case still stands, sometimes you don't want it to keep poping up. I can think of two ways to achieve this:

• Adding an option to the preferences windows. This would be the easiest, but it makes the behavior lose a little of flexibility (you can't do it "on the fly").
• Differentiate between clicking on the expander and some other gesture, and have different behaviors for each. For example, if clicking on the expander, then it's considered as manually hiden, and won't show up until manually expanded again; if using a keybinding, or a click on the focused album after a while, then it wouldn't be considered as manually hide and will show up again next time.

[jrbastien]

@asermax, I prefer the second option. But if you could consider the second click behavior that would be nice. This is a bit surprising to see the track list expands right when it gets the focus.

Edit: If you are afraid that the second click be considered as a double-click, there could be an option to set the time delay in the preferences.

[fossfreedom]

ok - I've hacked some stuff together - literally.

This commit introduces the following:

1. first click on album - just highlights as normal
2. second click on album - entry view opens
3. third click on album -entry view closes

Also if the entry view is open and you manually close via the expander - the entry view will not reopen again until you manually reopen via the expander.

This is the hack bit - it seems a double click GTK event will also first call the standard click event 2 times BEFORE the double-click event.

However, we need both functionality on both single click as well as playing from a double click.

Taking the suggestion from @jrbastien .. the single click event opens the entry view after a half-second delay.

The double click event then has time to register - this sets up the entry view to say "I'm now in double-click mode". By the time the single click "open entry view" kicks in, "I'm now in double click mode" is recognised - so the entry view opening ignores everything.

The double click event then introduces a one second delay to reset the entry view opening - thus after one second, the opening of the entry view operates as normal - e.g. responding to single click events as standard.

The half-second, one second figures are just arbitary values - these can be played with if necessary.

[jrbastien]

Thank you. I really like this mode because it is intuitive. I used to always look for the expander for a couple of seconds.

I have tested it and was not able to find any problem. Even the double-clicks still works very well.

But since I'm not your only customer, you might want to consider an option to turn off the auto-expand mode on click.

[fossfreedom]

thanks for the feedback.

Given its now a "two click" to open/close I dont think it really warrants an "option" to control this.

However - if we want a choice between a single click (as originally implemented) and a two click (as now) then yes I agree, would need a preference option.

For the moment, probably content to let this stay as currently implemented.

I'll sleep on this for a couple of days(!) before deciding if to merge this or work on it a bit more.

[asermax]

One question: whats the delay for? Also, when clicking on two different albums fast enough, it seems to be considered as a double click. I don't think that's the correct behavior tho.

EDIT: I gave the code a little more of reading and I've to say that it's pretty hard to follow. Adding time conditions like that for the whole thing to work makes it hard to understand and maintain; there isn't another way somewhat more straight forward to implement the same thing? In the case there isn't another way, I recommend using Gdk.threads_add_timeout or Gdk.threads_add_timeout_seconds instead of the custom async_call you introduced. It basically does the same thing, but more Gtk friendly.

[fossfreedom]

I've tried clicking between two albums fast - but cant reproduce this. I'm using the laptop trackpad though - maybe not fast enough?

The slight delay is to allow the double-click event to take precedence over the normal single-click event-view opening code - as I said above - when you double click (as you predicted) - the single click event is fired first. This leads to a double click playing an album as well as sometimes opening the entry-view automatically - sometime it flashes open and close.

The half second and 1 second delays are just arbitary - can be changed if these are impacting.

If there is a better way to detect that a double click has taken place whilst in the single click event - that would be better - then can remove all the delay and async stuff. But dunno how to do this.

[asermax]

What about saving the event time and comparing each time the mouseclick event is fired? For example:

# first time there will be no last_click_time; it must be set to None in the constructor
if self.last_click_time and curr_time - self.last_click_time <= click_delay:
    do_suff()

self.last_click_time = curr_time

EDIT: Oh, and about clicking different albums, you should be able to reproduce it, at least the code allows it to happen, since it doesn't check if the clicked album is the same as the previously clicked one xD

[fossfreedom]

hmmm - I'll have a play.

N.B. any ideas if there is a way to find out what the double click interval is set to?

[asermax]

According to the GDK documentation:

For a double click to occur, the second button press must occur within 1/4 of a second of the first. For a triple click to occur, the third button press must also occur within 1/2 second of the first button press.

Anyway, reading further onto the documentation for the GdkEventButton may give us a better way to do this. Here's another idea, based on this:

The type field will be one of GDK_BUTTON_PRESS, GDK_2BUTTON_PRESS, GDK_3BUTTON_PRESS or GDK_BUTTON_RELEASE.

We can expand the entry view when the event is GDK_BUTTON_PRESS and hide it if we recieve a GDK_2BUTTON_PRESS or GDK_3BUTTON_PRESS. That way, we would be effectively showing the entry view only when a single click is recieved, after it got focus.

Oh, and I did some testing and the thing about clicking two different albums seems to be an effect of the delays. It behaves something like what you explained, it shows and then hides, but the interval between is pretty noticeable in my case. If we succesfully change the approach for that, then this other issue will be fixed as well.

[fossfreedom]

Looks like the GDK_BUTTON_PRESS is sent on a double click, followed by a GDK_2BUTTON_PRESS etc.

If we open on a GDK_BUTTON_PRESS and close on a GDK_2BUTTON_PRESS then you get a really annoying flash as the entry view opens and closes :/

I've done some minor timing changes taking into account the Gdk .25 and .5 second figures. Also as suggested I've replaced the custom async with the Gdk equiv.

[jrbastien]

I was pleased to see this code in the master branch. I have noticed a small issue however:

If the cover is in the middle of where the list expands, it will auto-scroll to the upper part of the screen when clicking on it (this is the desired behavior)

If the cover is located in the bottom part of the screen, clicking on it to expand the track list will completely hide it. Is this the expected behavior?

[asermax]

@fossfreedom I made some changes around your code, hope you don't mind, I was trying to clean it up and fix some bugs I was suffering:

• This rapidly clicking between different albums kept causing various strange behaviours, the entry view hided or showed when it shouldn't have.
• If you use the keyboard to focus an album, a later click on the focused album should bring up the entry view.
• Again, my main objective was to remove this time conditions or at least try to make the whole thing more straight forward.
• The viewport scrolling behavior wasn't being taken into accound when expanding the entry view manually.

I introduced some work arounds myself, but I think the end result is a little clearer. Basically, the logic of the new code is:

1. When the mouseclick callback is called for the first time, increment the click_count by one and add a timeout to the _timeout_expand method (old _expand_and_scroll). Each subsequent call to the mouseclick callback would increment the click_count by one.
2. When the _timeout_expand method gets finally called, it haves to check two things: that the clicked album is the currently focused album and that the click_count hasn't went up. If this conditions are met, then we are sure that this is a second/third valid click, so we proceed to show/hide the entry_view.
3. Before returning, we make sure to update the currently selected album and clear the click count.

To knock out the keyboard movement issue and the rapidly clicking albums bugs, I had to do a somewhat dirty workaround on the selection changed callback:

if selected[0] is not self.last_selected_album:
    if not self.click_count:
        self.last_selected_album = selected[0]
    else:
        self.click_count -= 1

I'll explain the inner if-else since it's pretty confusing: the not self.click_count allows us to detect that the currently selection change was made through the keyboard, and hence we should update the last_selected_album or else the next click on that album won't know that that click should behave like a "second" click. On the else branch, reducing the click_count achives to correctly ignore double clicks when they aren't on the same album, and fixes the rapidly clicking albums issues.

Finally, some other things I moved around/deleted/changed:

• I moved the expand & scroll behavior to the expander callback, so it would be available ot either second/third click gestures or manual expansion.
• I removed the manual expanding flags, since that idea was actually if we were going to implement a single click expand instead of the second/third click behavior. It doesn't makes much sense in the current schema (if you don't want the entry view to expand, you won't go arround second clicking albums xD)
• Since most of the ExpandTypes where now unneeded (since I basically reduced it to two states and managed them through a local object variable), I also removed the related code.

Anyway, each time we introduce new stuff to the cover view (the quick_search and this one for instance) it gets even more crowded... we may have to start with the refactoring (chop the source module into various widgets) soon :T I believe that most of the behavior is pretty well delimited, so it shouldn't be too hard to isolate each important feature in it's own widget. I'll see if I start on that later today or tomorrow.

@jrbastien the behavior you describe seems to happens only in the last one or two rows of the coverview (I mean when it's scrolled all the way to the bottom), isn't that right? I tried to fix it but couldn't do anything about it :/

[fossfreedom]

@asermax - ta - good stuff - I've been preoccupied trying stuff with that damn spacing issue.

I'm going to copy the above into the wiki - useful stuff.

[jrbastien]

@asermax, yes the cover not scrolling only happen at the bottom of the coverview. Could you add an invisible cover row to provide room for the scrolling? You would need to remove it once the list is hidden again.

[asermax]

Adding an extra empty row would be troublesome to manage, and it wouldn't be tackling down the main problem either. I found out that the reason it was failing was because the signal of the bottom pane being expanded is sent before the size allocation is updated, then when we ask the cover view to scroll up to show the last row, it would ignore the request since it can't scroll down any further (since the cover view isn't aware yet of the size allocation change). I tried looking for another signal that we could use that would be fired after the bottom pane expansion, but couldn't find anything suitable, so I ended up queueing the scrolling to be done after the expansion takes place. In theory it should always work, but I'm not so sure, so let me know if you find anything suspicious.

[jrbastien]

Thank you @asermax for fixing this last issue. This works fine on my system now.