Merge lp:~elementary-dev-community/noise/fix-1089869 into lp:~elementary-apps/noise/trunk

Proposed by Cameron Norman on 2014-02-27
Status: Merged
Approved by: Corentin Noël on 2014-03-07
Approved revision: 1561
Merged at revision: 1560
Proposed branch: lp:~elementary-dev-community/noise/fix-1089869
Merge into: lp:~elementary-apps/noise/trunk
Diff against target: 47 lines (+15/-20)
1 file modified
src/Views/Wrappers/ViewWrapper.vala (+15/-20)
To merge this branch: bzr merge lp:~elementary-dev-community/noise/fix-1089869
Reviewer Review Type Date Requested Status
Corentin Noël 2014-02-27 Approve on 2014-03-07
Daniel Fore 2014-03-01 Approve on 2014-03-02
Review via email: mp+208528@code.launchpad.net

This proposal supersedes a proposal from 2014-02-27.

Commit message

Change view selector sensitivity to a more consistent state

Description of the change

This changes a couple things about the visibility and sensitivity of the search field and view selector.

1: the view selector is invisible if the list view or grid view is unavailable. This fixes #1089869 because the grid view is not available when the queue, device, et al lists are in use, and the view selector is made invisible.

2: the view selector and search field are invisible on the welcome screen (needs input from elementary UX).

3: the view selector is insensitive if the alert screen is present but the grid and list views are still available.

4: the column mode toggle is never invisible unless the whole view selector is.

To post a comment you must log in.
1560. By Cameron Norman on 2014-02-27

nitpick for consistency

Daniel Fore (danrabbit) wrote :

Hrm, I'm not sure I like having them both removed from the welcome screen. I would expect the search bar and view switcher to be present, but insensitive here since they can be enable for this "Source" (the music library) by adding music.

Removing them when you switch to another source makes sense because there isn't a way to enable them that that source. But, to me, it still feels kind of weird. I think I personally would rather the switcher just becomes insensitive in all cases instead of being completely removed.

review: Needs Fixing
1561. By Cameron Norman on 2014-03-02

going with insensitive for all situations

Cameron Norman (cameronnemo) wrote :

So I made those changes. I definitely agree about the welcome screen. I am not so sure about the other sources (for the view selector), though. The user is never going to be able to switch the view in those situations, so I would think it should be invisible. It does look a little strange, I admit.

Daniel Fore (danrabbit) wrote :

Much better

review: Approve
Corentin Noël (tintou) wrote :

It's okay for me, let's merge it !

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/Views/Wrappers/ViewWrapper.vala'
2--- src/Views/Wrappers/ViewWrapper.vala 2013-06-26 21:03:16 +0000
3+++ src/Views/Wrappers/ViewWrapper.vala 2014-03-02 02:17:58 +0000
4@@ -222,28 +222,23 @@
5
6 debug ("update_library_window_widgets [%s]", hint.to_string());
7
8- // Insensitive if there's no media to search
9-
10- bool has_media = media_count > 0;
11- App.main_window.searchField.set_sensitive (has_media);
12+ // Search field
13+ // Insensitive if there's no media to search (applies to ALERT/WELCOME views)
14+ App.main_window.searchField.set_sensitive (media_count > 0);
15
16- // Make the view switcher and search box insensitive if the current item
17- // is the welcome screen. The view selector will only be sensitive if both
18- // views are available.
19+ // View switcher
20+ // Insensitive if the current view is the welcome/alert screen or if both views
21+ // are not available (in the queue, device, or playlist sources).
22 App.main_window.viewSelector.set_sensitive (has_grid_view && has_list_view
23- && current_view != ViewType.ALERT
24- && current_view != ViewType.WELCOME);
25-
26- bool column_browser_available = false;
27- bool column_browser_visible = false;
28-
29- // Sensitive only if the column browser is available
30- column_browser_available = list_view.has_column_browser;
31- if (column_browser_available)
32- column_browser_visible = list_view.column_browser.visible;
33-
34- App.main_window.viewSelector.set_column_browser_toggle_visible (column_browser_available);
35- App.main_window.viewSelector.set_column_browser_toggle_active (column_browser_visible);
36+ && current_view != ViewType.WELCOME
37+ && current_view != ViewType.ALERT);
38+
39+ // Set active mode to column view if it is visible. We have to ensure
40+ // that it is not null because the column_browser is not guaranteed to
41+ // exist. This is done separately from below because of ViewSelector's
42+ // poor API.
43+ App.main_window.viewSelector.set_column_browser_toggle_active (list_view.column_browser != null
44+ && list_view.column_browser.visible);
45
46 // select the right view in the view selector if it's one of the three views.
47 // The order is important here. The sensitivity set above must be set before this,

Subscribers

People subscribed via source and target branches