-
-
Notifications
You must be signed in to change notification settings - Fork 74
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
fix(ui): never-ending loading animation on empty search #260
Conversation
Hi @compiuta, thanks for your proposal! I’m currently taking a 10-day vacation and try to relain as disconnected as possible from usual activities, such as reviewing pull requests. As I’m the only maintainer of this project who has an understanding of the whole codebase, and automated tests dont seem to pass anymore (for some reason), I will forge it a review when I’m back. If you want to help us save some time, feel free to propose a separate PR to fix the automated tests. Thank you for your understanding! Adrien |
Hey @compiuta, how are you? I'm sorry it's taking so long for me to review this... During my holidays, YouTube decided to shut down the API key we've been using for years. Because of that, several users reported that features relying on YouTube (e.g. adding tracks) have been failing. As this is quite a blocking issue impacting many users of Openwhyd, and as I've not found anyone else willing to fix this, I've been investing most of my spare time on these issues since yesterday. If you want more details about the situation and the fixes being put into place, feel free to have a look to the following tickets:
Thanks for your understanding and patience! |
No worries @adrienjoly I will take a look and help out if I can. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It works! Thanks for your contribution, @compiuta!
Alright glad I could help :) |
@compiuta, if you'd like to contribute again to Openwhyd and are not sure on what, feel free to let me know what kind of tasks you enjoy (and don't enjoy) doing so I can make some recommendations for you. You can reply here or contact me by email if you prefer. (adrien at openwhyd dot org) |
@adrienjoly anything having to do with the front end ui I can gladly help with. Definitely let me know if there is a specific issue that you would like me to look at. |
Sure! Would you rather:
|
I enjoy fixing bugs and let me know if you have a specific issue with some part of the current ui so I can take a look. |
Sounds good I will take a look and see what I can do 👍 |
* master: (64 commits) chore(release): 1.30.6 [skip ci] fix(ui): Resizing the page should not scroll to the top (#303) chore(release): 1.30.5 [skip ci] fix(tests): Migrate acceptance tests from Webdriver to Cypress (#301) chore(release): 1.30.4 [skip ci] fix(tests): create dummy posts from cypress tests instead of from initdb_testing (#304) chore(release): 1.30.3 [skip ci] fix(tests): Reset database between each Cypress tests (#302) chore(release): 1.30.2 [skip ci] fix(ui): Display correct track error message when using electron app (#294) chore(release): 1.30.1 [skip ci] fix(error): 404 redirect when clicking load more on account stream (#296) test(#295): add non-regression test for pagination (#299) chore(release): 1.30.0 [skip ci] feat(ci): separate ci tasks for each type of tests (#298) chore(release): 1.29.0 [skip ci] feat(ci): enable TypeScript in Cypress tests (#297) remove browser cache hack preventing css from displaying correct logo (#293) chore(release): 1.28.2 [skip ci] fix(ui): never-ending loading animation on empty search (#260) ... # Conflicts: # .github/workflows/nodejs.yml
Closes #226.
What does this PR do / solve?
The console javascript error appears because there is a results condition in the file app/templates/searchPage.html.
This condition wraps the tab_tracks element that the file public/js/searchPage.js is looking for. I found 2 front end solutions to this specific issue.
Overview of changes
First we can wrap the searchPage.js file within the same condition as the html and give some sort of html message stating an invalid search query has been inputed.
Or we can have a condition within the searchPage.js file checking for the element. If the element is not found then the page will redirect to the homepage. This is the technique I found being used by google and amazon when I added an empty query in the url. In this pull request I have used this second solution.
How to test this PR?
I used the javascript debugger and the console to add a javascript break before entering the condition. After a valid and empty search there were no errors showing and the page redirected successfully when an empty search was given.