GNOME Bugzilla – Bug 699848
Follow mockups for shuffle/repeat
Last modified: 2013-06-05 16:43:39 UTC
Mockups: https://raw.github.com/gnome-design-team/gnome-mockups/master/music/wire-shuffle-repeat.png To do items include: * Update the strings in the menubutton * Make sure that the menubutton icon updates to reflect the current state
*** Bug 699172 has been marked as a duplicate of this bug. ***
*** Bug 699171 has been marked as a duplicate of this bug. ***
Created attachment 245885 [details] [review] implement the repeat/shuffle menu design following the mock ups please review
Review of attachment 245885 [details] [review]: * missing icon when setting shuffle/repeat off option * shuffle/repeat off should be default (Allan, please confirm that)
Created attachment 246004 [details] [review] implementation of the design mockups for shuffle/repeat menu okay patch updated, after requesting the new icon from design team.
Review of attachment 246004 [details] [review]: ::: data/PlayerToolbar.ui @@ +358,3 @@ <property name="can_focus">False</property> <property name="margin_top">1</property> + <property name="icon_name">playback-pause-symbolic</property> Breaks pause icon ::: src/player.js @@ +307,3 @@ this._pauseImage.modify_fg(Gtk.StateType.ACTIVE,color); + let shuffle = Gtk.RadioMenuItem.new_with_label([],"Shuffle"); These element should be defined in .ui file (use PlayerToolbar.ui) as a) this clutters the code, b) renders the labels untranslatable
Review of attachment 246004 [details] [review]: ::: data/PlayerToolbar.ui @@ +301,3 @@ <property name="has_focus">False</property> <property name="is_focus">False</property> + <property name="icon_name">media-playlist-consecutive</property> Correct icon name is media-playlist-consecutive-symbolic ::: src/player.js @@ +411,3 @@ + + _onShuffleRepeatOffActivated: function(data) { + this.repeatBtnImage.set_from_icon_name('media-playlist-consecutive', 1); Correct icon name is media-playlist-consecutive-symbolic
Created attachment 246089 [details] [review] Implement the mockup design for repeat/shuffle menu all fixed
Created attachment 246092 [details] [review] implementation of the mockup design for shuffle/repeat menu
Review of attachment 246092 [details] [review]: Patch is generally OK, but previously menu items are not cleared after a new option is selected
Created attachment 246100 [details] [review] implementation of mockup design for shuffle/repeat menu
Thanks, works great! The following fix has been pushed: 8c8d9af implementation of mockup design for shuffle/repeat menu
Created attachment 246101 [details] [review] implementation of mockup design for shuffle/repeat menu