-
-
Notifications
You must be signed in to change notification settings - Fork 287
Feature/ability to see search result in index page's table #3712
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
base: main
Are you sure you want to change the base?
Feature/ability to see search result in index page's table #3712
Conversation
Code Climate has analyzed commit c7fa582 and detected 1 issue on this pull request. Here's the issue category breakdown:
View more on Code Climate. |
26d949a
to
7a5e338
Compare
This PR has been marked as stale because there was no activity 8000 for the past 15 days. |
Screen.Recording.2025-04-07.at.6.00.16.PM.mov@Paul-Bob @adrianthedev finally the search is working now, Please let me know what would nest step 😄 |
Issues with the branch
@Paul-Bob could you please take a look and guide me through |
Hi @SahSantoshh I'll have a look during this week, thank you for your patience! |
Hi @SahSantoshh I noticed that debounce was already working. I've just pushed a commit enabling pagination and URL update with the current search query! |
589a142
to
0510c89
Compare
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.
Amazing work here @SahSantoshh!
I've left some comments with questions and some tweaks, let's discuss on those.
I tested it on index and seems to work as expected, how does it behave on association fields, on a has_many for example?
turbo_stream.replace( | ||
"#{@resource.model_key}_list", | ||
partial: "avo/index/resource_table_component", | ||
locals: { | ||
resources: @resources, | ||
resource: @resource, | ||
reflection: @reflection, | ||
parent_record: @parent_record, | ||
parent_resource: @parent_resource, | ||
pagy: @pagy, | ||
query: @query, | ||
actions: @actions 8000 | ||
} | ||
), |
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.
Do we need to call the extra partial?
I wonder if we can use the partial code directly here, I give it a quick try and something was not working properly, could you please explore this a bit?
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.
yeah, tried some approach it didn't work. so pushing the changes a head
Issues:
|
View type is now fetched from the resource, |
@SahSantoshh, please re-request a review once the changes and discussion points have been addressed. |
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.
Thanks for your patience on this PR, @SahSantoshh.
There's one question I'd like to clarify, I'll quote it from the previous review:
I tested it on index and seems to work as expected, how does it behave on association fields, on a has_many for example?
I've also left a comment on one of the discussions from the last review:
#3712 (comment)
4a25fef
to
e44936b
Compare
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.
@SahSantoshh I already left a review, let me know if you have any question
Main question is:
|
Sorry, I missed this one. |
@Paul-Bob could you please let me know some more details to deal with search for |
First, we need to verify the URL's proper transmission from the stimulus controller, I suspect it is not correctly seted for associations. Please review the previous search component's URL computation method for reference. |
ExecutionContext
# Conflicts: # app/controllers/avo/base_controller.rb
1f9aa69
to
56b4371
Compare
Hi @Paul-Bob I am working on the resource_association search and its not working as expected. Could you please take a look? |
Hi @SahSantoshh , yes, thanks for letting me know |
Description
Fixes 2842
Checklist:
Screenshots & recording
Manual review steps
Manual reviewer: please leave a comment with output from the test if that's the case.