search

joshghent / gifbar

🦄 Find Awesome Gif's right in your Menu Bar
MIT License
42 stars 29 forks source link

Search bar is hidden when no image results are found #100

Closed jamescallumyoung closed 5 years ago

jamescallumyoung commented 5 years ago

The problem

When a search is made, and no results are found, the page is replaced with a gray view that shows just a loading icon.

When this happens, the search bar is covered so the user cannot alter the search query. This results in the app becoming unusable; the user cannot alter the query to get new gifs.

The only way to get out of this situation seems to be to quit the app and restart it. Closing the window has no effect.

How to reproduce

  1. Open the app
  2. Enter a search query that will return no results
  3. The user is not trapped on the loading screen

Proposed solution

The search bar should always be visible.

Additionally, when the search query changes, the current gif request should be abandoned and the new query should be sent. (This allows the user to recover from a "no results" situation.)

When no results are found, a message should be shown. The current implementation suggests that the request has not yet returned (it still shows the loading bar).

jamescallumyoung commented 5 years ago

@joshghent, I found this when looking into adding support for multiple APIs for #11.

Could you also add the "help-needed" and "hacktoberfest" labels? This might encourage others to get involved.

joshghent commented 5 years ago

Great bug report! 🐛 Thanks for the write up - feel free to tackle it if you get the time 👍

TheMightyPenguin commented 5 years ago

Hey @jamescallumyoung @joshghent I'd like to tackle this if you guys haven't started working on it! 😄

joshghent commented 5 years ago

@TheMightyPenguin Go for it mate! All yours would really appreciate your help! 👍

jamescallumyoung commented 5 years ago

@TheMightyPenguin Great! I've not started on it so feel free to do so. Good luck 😄

zolomohan commented 5 years ago

This happens because, when no results are found, the state is set to an empty array. In the gifBox component, when the state is empty, it renders only the Spinner component. The search field is in the gifList component. This could be fixed by moving the search field to the gifBox component and rendering it always, since we always need the search field.

zolomohan commented 5 years ago

I've sent a PR #103 fixing the issue.

zolomohan commented 5 years ago

@joshghent , BTW i don't know how to write a test. I've moved around some things so the existing test keeps failing. I tested it manually by using the components and it work's fine. I hope someone writes test for it. I've attached some screenshots to prove it's working in my PR #103 .

joshghent commented 5 years ago

Thank you @zolomohan! Awesome work! Since the extensive changes in #106 by @jamescallumyoung - this PR may need some modification but happy to tackle this myself as you've already done the leg work 👍

zolomohan commented 5 years ago

I've resolved the merge conflicts with the master branch. Review it and the PR is good to go.

joshghent commented 5 years ago

Fixed in #103 - Thanks a lot @zolomohan 🎉

zolomohan commented 5 years ago

You're Welcome @joshghent 😀