-
Notifications
You must be signed in to change notification settings - Fork 102
Filter and search recipes on the main page #318
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
Filter and search recipes on the main page #318
Conversation
| ['name' => 'main#home', 'url' => '/home', 'verb' => 'GET'], | ||
| ['name' => 'main#keywords', 'url' => '/keywords', 'verb' => 'GET'], | ||
| ['name' => 'main#categories', 'url' => '/categories', 'verb' => 'GET'], | ||
| ['name' => 'main#category', 'url' => '/category/{category}', 'verb' => 'GET'], |
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.
Is this really no more needed? I just found this line but I am not sure if this is related (not JS/VUE expert). Just the red warn lamp when changing the API.
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.
The API is, at the moment, a mix of the old PHP routes (which are reachable only through manually writing them on the address line) and the new JSON API routes. I have not just gone ahead and pruned it because I am not familiar with the back-end and don't want to remove anything that is actually still in use. Unless the goal is to maintain both the Vue front-end and a backup synchronous front-end for people who cannot or refuse to use javascript, the obsolete routes should be removed at some point. Maintaining two front-ends is too big an undertaking for me at least, so unless there is someone else on the team to maintain the PHP template side, I suggest we get rid of it sooner rather than later. Again, this should probably have its own issue.
And yes, the line you spotted in the Vue router does replace the line that was removed.
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.
In fact, you moved the routes to the /api prefix here. This is perfectly fine for me. I was just curious why in the vue router the prefix is not mentioned.
|
Additionally, you might want to amend the latest commit and sign it off (by Regarding the pure VUE part I am not able to help much at the moment. I would have to check out the version and test it manually. Unfortunately, I have no dev environment at hand at the moment. I might have a look next week. please ping me if you need my help and I forgot to do so. |
I don't know how this can be missing, possibly something has gone wrong with my VSCode settings. I'm away from my dev environment as well, but will resolve this next week. |
Signed-off-by: sam-19 <[email protected]>
|
Ok, I started by doing a local merge with my most up-to-date local branch. Regarding the filtering I see the following bug: If you apply a filter, close the small overlay window (you created with the filter and search) and reopen it, the filter term is empty. So to clear the filter you have to type something and remove it afterwards. The search seems not to be working. I am not (yet) aware, where the problem lies. I get always an empty result list from the backend. Further debugging is needed. |
|
I found the cause for the empty answer from the backend and corrected things accordingly. |
This should be fixed now. |
|
@sam-19 I made some manual changes in order to make this PR mergable. Would you rebase and amend the commits of yours to get them signed off ( |
|
Okay, I tried to amend the commits by rebasing, which went to hell as usual. Now the rebase is stuck in an error state that I cannot resolve. I'm afraid this PR may be lost. I will pick the file changes into another PR sometime later when I have time. |
|
I just reverted your rebase as I had the original ones still at hand. If you are still mid-rebase, yopu can abort it via |
…ins needed Signed-off-by: Christian Wolf <[email protected]>
Signed-off-by: sam-19 <[email protected]>
…extcloud-cookbook into filter-n-search-recipes
…ecipes Signed-off-by: Christian Wolf <[email protected]>
|
I rebased and signed off on your behalf. I hope this was ok for you. |
|
Thanks, that is completely ok. I don't know if it is a problem with VSCode's integration to git or what, but my rebases often get stuck in a loop when there is a merge conflict, and there seems to be no no way around it. |
A filter and search feature for the main page. Both fields are found under a three dot menu on the breadcrumbs row. The features pretty much do what they say; filter just filters the results already on the screen, search runs a new query to the database. As to why there are two functions: the three dot menu requires at least two elements and I could not find any way to display just one field on the breadcrumbs row. If having two fields is too confusing, the feature has to be redesigned (the field moved to the left side panel, for example).
I'll wait for your for feedback.
Fixes #243