Skip to content
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

Flexible file widget #3807

Merged
merged 36 commits into from
Feb 7, 2025
Merged

Flexible file widget #3807

merged 36 commits into from
Feb 7, 2025

Conversation

koopmant
Copy link
Contributor

Add the flexible file widget. The widget is not yet used on any form.

code based on flexible image widget
@koopmant koopmant requested a review from amickan January 30, 2025 10:27
@koopmant koopmant self-assigned this Jan 30, 2025
@koopmant
Copy link
Contributor Author

@amickan do you think these tests are enough? I've adapted existing tests from the FlexibleImageWidget but I'm not sure if I'm missing something obvious. I'll take another look at it tomorrow.

@amickan
Copy link
Contributor

amickan commented Jan 31, 2025

@amickan do you think these tests are enough? I've adapted existing tests from the FlexibleImageWidget but I'm not sure if I'm missing something obvious. I'll take another look at it tomorrow.

The tests look comprehensive to me. The only thing that might be worth an additional test is the search view (see also the comment I left about the search fields). I don't think we have a test for the image search view, maybe they can be combined?

@koopmant
Copy link
Contributor Author

koopmant commented Feb 3, 2025

The tests look comprehensive to me. The only thing that might be worth an additional test is the search view (see also the comment I left about the search fields). I don't think we have a test for the image search view, maybe they can be combined?

We do have a test for the image search view, just found it after writing a test for the file search results view. I did not combine at the moment... I can pick that up when I will reduce the duplication between the two.

@koopmant koopmant marked this pull request as ready for review February 4, 2025 08:48
@koopmant koopmant requested a review from amickan February 7, 2025 10:06
Copy link
Contributor

@amickan amickan left a comment

Choose a reason for hiding this comment

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

LGTM

@koopmant koopmant merged commit 87af132 into main Feb 7, 2025
8 checks passed
@koopmant koopmant deleted the flexible-file branch February 7, 2025 11:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants