Skip to content

Conversation

@sinisaos
Copy link
Member

Related to #466

@codecov-commenter
Copy link

codecov-commenter commented Dec 19, 2025

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.04%. Comparing base (1157c12) to head (e299112).
⚠️ Report is 50 commits behind head on master.
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #469      +/-   ##
==========================================
+ Coverage   93.42%   94.04%   +0.62%     
==========================================
  Files           5        5              
  Lines         365      403      +38     
==========================================
+ Hits          341      379      +38     
  Misses         24       24              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Member

@dantownsend dantownsend left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this. It's cool how it syncs the filter values to the URL params.

But I think the missing piece is the reverse - so if you copy the URL, and post it into a new tab, it should then take the URL params, and populate the filter sidebar.

@sinisaos
Copy link
Member Author

sinisaos commented Dec 20, 2025

But I think the missing piece is the reverse - so if you copy the URL, and post it into a new tab, it should then take the URL params, and populate the filter sidebar.

But it doesn't work with or without PR changes (with current code in main branch) even in the same tab. As soon as we close the filter sidebar (and open again), the form fields are empty.
EDIT: With the latest commit filter form fields are populated from query params (in the same or different browser tab). Input fields works but select is not working and I don't know how to fix it.

@dantownsend
Copy link
Member

@sinisaos That's cool, thanks! I've tested it locally, and it works great, besides the select being populated, as you mentioned. I haven't been able to figure that out yet.

@abhishek-compro
Copy link

abhishek-compro commented Dec 22, 2025

select is not working

@sinisaos Maybe because of this where 'e' (equals) is a hard coded value?

@sinisaos
Copy link
Member Author

@abhishek-compro That makes sense, but it doesn't really affect anything. If I remove that line entirely, the result is the same because e (Equals) is first item in operetors map. Each field in the filter form has

  1. a select for column__operator or column__match (based on the column type)
  2. the actual value of the field

I've experimented with ChoiceSelect by adding localValue (not value) and that populates the select, but now I'm having trouble with null values. Then there are still the Array fields. This is not an easy task.

@sinisaos
Copy link
Member Author

sinisaos commented Dec 22, 2025

@dantownsend Now the select works as far as I can test and see (please try it). The selects are populated correctly from the query params. The ArrayWidget also works. Only the FK field with KeySearch does not work.

filter_query_params.mp4

@dantownsend
Copy link
Member

@sinisaos This looks great, thanks! I've done some testing, and it seems to work for all field types now?

The only thing I'd like to add in a future PR is also syncing the page number to the query params too.

@sinisaos
Copy link
Member Author

@sinisaos This looks great, thanks! I've done some testing, and it seems to work for all field types now?

Yes. It should work for all field types.

The only thing I'd like to add in a future PR is also syncing the page number to the query params too.

Yes, that would be nice. I can try to do that, but you need to merge this PR and the pagination PR first, because there's no point in trying to make those changes now, if the pagination is going to change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants