Audience

Merge lp://staging/~artem-anufrij/audience/library-search into lp://staging/~audience-members/audience/trunk

library-search
Merge into trunk

Proposed by Artem Anufrij on 2016-09-23

Status:

Merged

Approved by:

Cody Garver on 2016-09-23

Approved revision:

673

Merged at revision:

661

Proposed branch:

lp://staging/~artem-anufrij/audience/library-search

Merge into:

lp://staging/~audience-members/audience/trunk

Diff against target:

333 lines (+130/-37)

3 files modified

src/Widgets/LibraryPage.vala (+55/-17)
src/Widgets/WelcomePage.vala (+1/-3)
src/Window.vala (+74/-17)

To merge this branch:

bzr merge lp://staging/~artem-anufrij/audience/library-search

Related bugs:

Bug #1624940: Keyboard navigation doesn't work	Medium	Fix Released
Bug #1626712: Search library	Wishlist	Fix Released

Link a bug report

Reviewer	Review Type	Date Requested	Status
Danielle Foré	ux	2016-09-23	Approve on 2016-09-23
Review via email: mp+306660@code.staging.launchpad.net

Commit message

Add library search support and fix keyboard navigation

lp://staging/~artem-anufrij/audience/library-search updated on 2016-09-23

662. By Artem Anufrij on 2016-09-23: add missing file
663. By Artem Anufrij on 2016-09-23: push after merge:audience for newest version

Revision history for this message

Danielle Foré (danrabbit) wrote on 2016-09-23:

* Pressing `esc` should probably clear the search

* Is there a reason to put the search in a revealer instead of in the header? With the revealer, the entry is really far away from where you clicked to show it. It's a bit disjointed feeling.

* The hint text should probably be "Search Videos" instead of "find"

Revision history for this message

Danielle Foré (danrabbit) wrote on 2016-09-23:

If there are no results, there should be a "No Results for 'foo'" screen. See Slingshot or Switchboard for an example

review: Needs Fixing

lp://staging/~artem-anufrij/audience/library-search updated on 2016-09-23

664. By Artem Anufrij on 2016-09-23: better filtering
665. By Artem Anufrij on 2016-09-23: searchbox goes headerbar
666. By Artem Anufrij on 2016-09-23: ctrl + b for browse library

Revision history for this message

Danielle Foré (danrabbit) wrote on 2016-09-23:

If I start typing in the library view, keyboard focus should automatically move to the search bar

lp://staging/~artem-anufrij/audience/library-search updated on 2016-09-23

667. By Artem Anufrij on 2016-09-23: alert view added
668. By Artem Anufrij on 2016-09-23: improved ctrl+f behavior
669. By Artem Anufrij on 2016-09-23: deleted redundant code
670. By Artem Anufrij on 2016-09-23: instant search on typing in library view

Revision history for this message

Danielle Foré (danrabbit) wrote on 2016-09-23:

Since this branch is also supposed to fix keyboard nav, it seems like pressing enter on the welcome doesn't activate the selection (but I don't think fixing that should block this branch if it's difficult)

It looks like the searchbar is also grabbing special keys like arrows. It should probably only grab alphanumeric and symbols

lp://staging/~artem-anufrij/audience/library-search updated on 2016-09-23