After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 699884 - Don't hard code button heights
Don't hard code button heights
Status: RESOLVED FIXED
Product: gnome-music
Classification: Applications
Component: general
unspecified
Other Linux
: Normal normal
: ---
Assigned To: Guillaume Quintard
gnome-music-maint
Depends on:
Blocks:
 
 
Reported: 2013-05-07 22:17 UTC by Allan Day
Modified: 2013-05-12 15:53 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
change the stringifying function (822 bytes, patch)
2013-05-08 10:46 UTC, Guillaume Quintard
rejected Details | Review
Let button fill the height (26.29 KB, patch)
2013-05-12 10:56 UTC, Guillaume Quintard
rejected Details | Review
prepare playground for Seif's patch (2.76 KB, application/octet-stream)
2013-05-12 14:40 UTC, Guillaume Quintard
  Details

Description Allan Day 2013-05-07 22:17:43 UTC
This commit hard coded the button heights for the play/pause, next, previous and shuffle/repeat buttons:

https://git.gnome.org/browse/gnome-music/commit/?id=d4fd52cf03a3ae7e161ef12830ba98373e407b64

We should be relying on the theme to provide the button height, not setting it manually.
Comment 1 Guillaume Quintard 2013-05-08 10:46:48 UTC
Created attachment 243573 [details] [review]
change the stringifying function
Comment 2 Guillaume Quintard 2013-05-08 10:47:41 UTC
arg, wrong ticket.
Comment 3 Vadim Rutkovsky 2013-05-08 11:11:38 UTC
Review of attachment 243573 [details] [review]:

Wrong bug
Comment 4 Guillaume Quintard 2013-05-12 10:56:11 UTC
Created attachment 243897 [details] [review]
Let button fill the height
Comment 5 Seif Lotfy 2013-05-12 11:56:16 UTC
This is how it used to be. And the designers changed it because the fill height was too much. :(
Comment 6 Shivani Poddar 2013-05-12 11:57:27 UTC
Review of attachment 243897 [details] [review]:

The patch works for correcting the button sizes , http://img.ctrlv.in.s3.amazonaws.com/img/518f82f22d29c.png , maybe because I am not using jhbuild, the separators are still there for me (although I see they have been chucked off the code) 
Still needs some jhbuild user for music to test it to be sure. :)
Comment 7 Seif Lotfy 2013-05-12 12:10:54 UTC
your screenshot does not show the bottom bar. and this is supposed t0 affect the bottom bar. Also when testing and showing screenshot make sure you are using Adwaita
Comment 8 Seif Lotfy 2013-05-12 12:11:40 UTC
Review of attachment 243897 [details] [review]:

As I said this brings us back where we started. The buttons are not too high
Comment 9 Guillaume Quintard 2013-05-12 14:01:55 UTC
(In reply to comment #5)
> This is how it used to be. And the designers changed it because the fill height
> was too much. :(

Ok, so the question is, what is the correct class we should apply to the buttons/playbar? Or even, is there a class fitting our purpose?
Comment 10 Guillaume Quintard 2013-05-12 14:40:41 UTC
Created attachment 243917 [details]
prepare playground for Seif's patch