[amarok] src: Refactoring: Using the same delegate almost everywhere in the browser

Ralf Engels ralf-engels at gmx.de
Tue Feb 26 13:36:51 UTC 2013


Am Dienstag, 26. Februar 2013, 13:29:53 schrieb Matěj Laitl:
> On 22. 2. 2013 Ralf Engels wrote:
> > Git commit 730583c601404719102ba92f72d07ab9ae14ade1 by Ralf Engels.
> > 
> > Refactoring: Using the same delegate almost everywhere in the browser
> 
> After using Amarok a couple of days with this patch I must say it was worth
> the effort. I have found another round of minor leftover problems however:
> 
>  1. The {category,collection,playlist provider} icons on the left seem to be
> scaled to some non-predefined size, which makes them look blurry here.
> After closer investigation it seems the icons are 28 pix while the natural
> size would be 32 pix. The action icons (plug in USB Stick for
> UmsCollection) seem to be upscaled too.
> 

The icon list icon size is default 28px.
Only the toolbar icon size is 32px. I thought about using this size but it 
seemed to me to be on the side of a dirty hack.
We were using a much to large icon size all along.

Changing this is easy, but I thought that this was the propper way.

>  2. The expansion indicator arrow on the right seems to have too little
> margin from the right border. This doesn't seem to come from the style as
> the expansion indicators for example Artist have some margin from the left
> border. Also, the "collapsed" state of the indicator seems horizontally
> flipped to me, but I realize this may be intentional (because it is on the
> right).

The thing with the expansion indicator: This is again a little dirty as the
style conform symbol might be a plus or a minus where we are always using
an arrow.

However the position of the arrow is correct. For debugging reasons I 
switched to right-to-left and there the arrow aligned perfectly with the
rest of the list.

The horizontal flipping really was intentional. While debugging right-to-left I 
noticed that we had it flipped around.

> 
>  3. In Playlists -> Saved Playlists, it lists playlist-related actions for
> playlist providers "Amarok Database" and "Playlist Files on Disk". This
> seems to be the problem of the model, not the delegate though. Clicking the
> actions also has a "funny" effect, the model seems to collect & combine
> actions from all child playlists. I will fix this unless you are faster.
> 
I haven't noticed that.
Also I tried to provoke the small-icon actions for playlists to see if they 
are still working correctly but I couldn't.

>  4. PrettyTreeDelegate.cpp:81:15: warning: unused variable ‘tinyIconSize’ [-
> Wunused-variable]
> ^^^ this may be a sign of a little bug when computing icon sizes.
> 

I already intended to fix all our warnings. Will do that too.

Ralf

> Regards,
> 		Matěj


More information about the Amarok-devel mailing list