Reviewing merge request #122: My up-to-date changes

Sorry, but the "please make merge request" request came too late and i wasn't in the mood to sort things out.

You may try and comment.

screenshot:
http://img196.imageshack.us/img196/7884/amarok20100215b.jpg

changes:
- well, ... skip buttons
- thus "#if'd" all sliding
- the "dialing" region of the dial is slight bigger
- the dial uses a conic gradient to stress the dial nature (whatever we'll end up with the dial, it probably not makes anything worse)
- kicked the "gradient" and added a "border" against the menu/titlebar instead (please believe me: gradients on the oxygen deco either make a bricky layout -strong border- or are superflous -unnotable- or look weird -i'd have to explicitly invoke the oxygen light structure, breaking them more or less everywhere else... this is not gonna work. period. make one if you can.)

PS: atm (15.02.2010 eve CET) i apparently cannot mail to amarok-devel. no bounces either... :-\

Commits that would be merged:

Version 6
  • Version 1
  • Version 2
  • Version 3
  • Version 4
  • Version 5
  • Version 6
  • Version 6
  • b1736ff
  • Thomas Lübking
almost 2 years ago
  • a4203a4
  • Thomas Lübking
almost 2 years ago
  • 73b407b
  • Thomas Lübking
almost 2 years ago
  • 74d52fa
  • Thomas Lübking
almost 2 years ago
  • 1333bc9
  • Thomas Lübking
almost 2 years ago
  • df963e0
  • Thomas Lübking
almost 2 years ago
  • 857bb75
  • Thomas Lübking
almost 2 years ago
  • 502294c
  • Thomas Lübking
almost 2 years ago
  • 2ddf09a
  • Thomas Lübking
almost 2 years ago
  • 0e82cd8
  • Thomas Lübking
almost 2 years ago
  • 4dd126f
  • Thomas Lübking
almost 2 years ago
  • 746f987
  • Thomas Lübking
almost 2 years ago
  • f720912
  • Thomas Lübking
almost 2 years ago
  • 463dbcd
  • Thomas Lübking
almost 2 years ago
  • 0e7f830
  • Thomas Lübking
almost 2 years ago
Showing 9e8cafb-b1736ff

Comments

This seems to work well, thanks! :–)

A few comments/ideas:

  1. I am not sure I like the “border gradient” much. The issue for me is that it visually adds 2 more horizontal lines in a relatively small vertical space (my eyes perceive a line at the top against the menu bar, but also, when causally glancing, a line at the bottom of the gradient, then there is a small gap, then there is a line at the top of the slider, a line at the bottom of the slider and a line at the end of the toolbar). To my eyes, this is very “busy” or crowded.

  2. Would it not be possible to maintain sliding by painting the next/prev icons as part of the background, and then quickly fading them out when sliding begins and fading them back in when sliding has stopped?. Or even just leaving the arrows in place and sliding the text over them? In any case, these are just wild ideas and there might be good reason why this is not possible or feasible.

  3. Nikolaj

1) I suppose it’s better than the one before. Personally i'd go w/o app side gradients (that’s really a style job and this gradient looks crap with e.g. plastique, though i could probably improve that.)
And yes: KDE has far too many (horizontal) lines. No doubt about that ;-P

2) I wrote that in the mails that didn’t make it to the list 4 times… :–<br/> Basically: no.
– Assuming ppl. will click the icon in most cases, the reaction should not be that it disappears. Period. :)
– Dragging across the icons look crap.
– As for sliding
: with the current translucent appearance of the icons and regarding the anim speed, this would be visually on the edge, but it’s a rather “weird” to click an arrow pointing to the right and things move to the left :–(

Also see the “Viral Marketing” mail regarding this >–)

*easy to test, there’re atm two preprocs to handle this

I agree with nhn, the gradient really rubs me the wrong way. it adds an arbitrary line under the titlebar that really goes against the oxygen feel.

also, what do you think about moving the labels/arrows back to the sides a little bit? i think you moved them in closer to the center and in my opinion that looks a bit weird now.

All labels currently share the same with, the width for the icons is “taken away” from the prev/next labels.

I oppose to have the icons jump around as well as to put floating space between them and the labels (thus the re-alignment r/c/l)

Furthermore the icons (arrows!) should point the tracklabels (i.e. attach the stopper) and not follow them.

Long story short:
aside from larger paddings, we could just steal arbitrary space from the nex/prev labels and pass it to the current label. No idea whether this is justified.
But with this layout, i see no way to anchor the labels to the slider (as it has been)

why not align the end of the track label text to the beginning and end of the slider?

“I oppose to have the icons jump around as well as to put floating space between them and the labels (thus the re-alignment r/c/l)”

Aligning to the outer edges would cause either and aligning to the slider ends would (in case of no moodbar, and the slider’s not gonna become a splitter then) also artificially cap the labels.

This is one of the drawbacks of these icons. Suck it ;-P

I like this screenshot.. but what about leo’s comment:

why not align the end of the track label text to the beginning and end of the slider?

As leo’s using the moodbar and thus the slider reaches from end to end for him, he probably meant to leftalign the prev and rightalign the next track.

The consequece would either be floating space between the skip icon and the label (i.e. visual detachment, the icon could easily be connected to the current track and you got 5 UI elements instead of 3) or that the icons attaches to the label and
a) jumps around (bad usability)
b) visually breaks center alignment (bad look)

The only way to get the prev/next labels away from the center is to make them shorter and the current track longer (what could lead to an external alignment to the slider w/o the moodbar, but not w/ …. plus quite short labels ;–)

Please notice that this is a “lousy” shot, both outer labels have fairly short content, my “next” just says “Wo ist Brünhild', wo die Verbrecherin” and covers the entire width :D

i don’t understand what moodbar has anything to do with it—-i don’t have any tracks with mood files, and there are no progress bars being drawn with moods.

i'm having some trouble parsing your english, so i apologize if I don’t quite understand what you’re saying. are you saying the problem you have is that if the labels are aligned to the left and right ends of the progressbar, respectively, the arrows would not always been in the same place each time (since the next/prev track text would change)?

we could also put the arrows on the outside of the text rather than on the inside. I know that you like the arrows “pointing to” the next track, and I agree it is a nice touch, but I think even if it is like this:

<–[prev song]—————– [cur song]——————–[next song]–>
(===================================)

(ignore the ——–, gitorious is ignoring whitespace so I needed a character to separate the track items)

it’s very very clear to the user what the arrow icons mean. after all, this is a media player, and left/right arrows mean exactly 1 thing when they are in the toolbar of a media player: next and previous track.

I apologize for misrecalling you to use the moddbar – wrong dev :–}
I also apologize for some lousy english.

the arrows would not always been in the same place each time (since the next/prev track text would change)?
precisely.

we could also put the arrows on the outside of the text
a) the labels had originally been aligned this way (though for other reasons) and it was explicitly demanded (see ML) to put them towards the center
b) this way you get the play and the prev “button” side on side on the left and thus the next “button” out of sight on the far right :–(
c) while putting the icons on the outer edges would diminish their visual imapct and bring the labels to the foreground (good ;–), the connection between label and icon would be lowered or lost (bad)
Also the implication of the skip typical “|” would be lost (thus we might have to remove it)

Here’s a screenshot and i'm willing to update the request, but please notice that this is not “bring back the god damn…” and might therefore cause another stir (or two) in the waterglas – well, not my fault ;-P
http://img685.imageshack.us/img685/9821/amarokarrows4.jpg
(it’s btw. the karajan recording… :)

Another side effect could* be, that this reminds to tabbars, which trigger no change on scrolling by clicking the icon.

*you really need to test this in the wild, sorry :–(

another thing:
i'd really like to remove the (stupid) [prefix]/[suffix] dummies.

at least as there’re arrows this should really not be necessary to hint the label nature.

objections?

yeah the prefixes should go away. with the arrows it’ll be really clear.

I like the new screenshot—-i think it’s a good improvement over what we have now. i think you should go ahead and merge, and we’ll deal with what comes next as it comes.

→ State changed from Open to Verifying

I'm going to try this locally here now.

Considering what Leo said (which makes sense IMHO), I think I'm going to go ahead and merge this into mainline, if it works out well.

Ok, let’s try it out now…

→ State changed from Verifying to Merged

So, I've tried it out here. I think it’s a step forward.

One issue: I think the Next/Prev items are now too close to the Play/Pause and Volume buttons. I would suggest adding some padding in between there to make it look more pleasant.

Overall: Great work, Thomas. Thanks :)

I'm merging it into mainline now, with the intention that we can build upon this patch, like Leo suggested.

Add a new comment:

Login or create an account to post a comment

How to apply this merge request to your repository