Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

App crashes with the newly added search feature #95

Closed
csoni111 opened this issue Mar 17, 2017 · 6 comments
Closed

App crashes with the newly added search feature #95

csoni111 opened this issue Mar 17, 2017 · 6 comments

Comments

@csoni111
Copy link
Member

The search feature implemented in #36 by @octacode is a great addition but unfortunately it has some bugs.

  • To reproduce the bug first type something in the search bar and then with the search query typed in, click on a directory to open its content. Now if you type anything more in the search bar, the app crashes.

  • Also instead of adding 5 images for different sizes, a single vector drawable can be added.

I would like to send a pr for resolving this bug.

@octacode
Copy link
Member

octacode commented Mar 17, 2017

@csoni111 thanks for reporting the bugs. Please go ahead.
However I remember solving the first bug I guess I did something bad while Resolving merge conflicts.
What I did was that I called the closeSearch() function in the ItemClick:-
https://gist.github.com/octacode/d014f1a02a9208fc9d657732a74a289f
Here and yes replacing the images with a single Drawable is really an awesome Idea. It'd help reduce the size.

@csoni111
Copy link
Member Author

csoni111 commented Mar 17, 2017

This helps! Also I have some other code changes in mind to further improve the search functionality. Will make a pr for all those.

@doomers
Copy link
Contributor

doomers commented Mar 17, 2017

@csoni111 see #30 @lakshyagupta21 is already working on it, the Vector Drawable part

@csoni111
Copy link
Member Author

Yes I know that, I will just update the search icon that has been added recently. He might not have pulled this change.

@doomers
Copy link
Contributor

doomers commented Mar 17, 2017

Okay.

@csoni111
Copy link
Member Author

@octacode I have made the pr. Please review it if possible!

@cpg cpg closed this as completed in 24ff288 Mar 18, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants