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

Add option to hide files in global media library #2035

Merged
merged 1 commit into from
Feb 18, 2023

Conversation

JoeyStk
Copy link
Contributor

@JoeyStk JoeyStk commented Jan 30, 2023

Short description

This PR was made in collaboration with @bob606 . This PR is supposed to add the option to hide files or directories in the global media library for regional media libraries.

Proposed changes

  • Update models for directory and media file, and the according migration file
  • Add checkbox in the view

Side effects

This PR is still a work in progress. So far the functionality that the directories and files actually get hidden is still missing and we have worked so far only on the prerequisites for it.

Resolved issues

Fixes: #1578


Pull Request Review Guidelines

@codeclimate
Copy link

codeclimate bot commented Jan 30, 2023

Code Climate has analyzed commit 7300c9f and detected 0 issues on this pull request.

The test coverage on the diff in this pull request is 100.0% (50% is the threshold).

This pull request will bring the total coverage in the repository to 75.1% (0.0% change).

View more on Code Climate.

@JoeyStk JoeyStk force-pushed the feature/hide_files_in_global_media_library branch 2 times, most recently from 6002932 to 7780fd3 Compare January 30, 2023 13:13
Copy link
Member

@MizukiTemma MizukiTemma left a comment

Choose a reason for hiding this comment

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

Thank you for the PR 👍

To save the value is_hidden I suggest to set id and name for the checkbox.

If I've understood the context correctlly, hiding folders matters only for folders of the global media library and we don't have to show this choice for folders of region media library (and let it stay always false).

@JoeyStk JoeyStk force-pushed the feature/hide_files_in_global_media_library branch 8 times, most recently from 9fd8154 to 1bff28d Compare February 6, 2023 12:38
@JoeyStk JoeyStk marked this pull request as ready for review February 6, 2023 12:50
@JoeyStk JoeyStk requested a review from a team as a code owner February 6, 2023 12:50
Copy link
Member

@MizukiTemma MizukiTemma left a comment

Choose a reason for hiding this comment

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

Thank you for PR!

I think it's working now to save whether the media file/folder shall be hidden in the media library 👍

There is one bug though: in region media library, when you click a file, another one is shown in the side bar (see the screen shots below).

I guess the cause is that the file index is not correctly set.

Screenshot 2023-02-07 145017
Screenshot 2023-02-07 145052

@JoeyStk JoeyStk force-pushed the feature/hide_files_in_global_media_library branch 6 times, most recently from 67fe1ca to 7b1e6b7 Compare February 9, 2023 12:12
@JoeyStk
Copy link
Contributor Author

JoeyStk commented Feb 9, 2023

@MizukiTemma Thank you for your suggestions. I wasn't able to reproduce your issue, but if your suggestion works, it should be fixed now. Can you please check?
If so, we can move to the next issue, the thing with the checkbox that is showed, although it shouldn't

Copy link
Member

@MizukiTemma MizukiTemma left a comment

Choose a reason for hiding this comment

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

Thank you!
It looks good: the checkbox is only in global media library available (hidden in region media library) and the correct data is rendered in the side bar 👍 The choice of hide/show is preserved also after replacing the file.

I have one small suggestion to make the description of the checkbox more self-explaining.

integreat_cms/cms/views/media/media_context_mixin.py Outdated Show resolved Hide resolved
@JoeyStk JoeyStk force-pushed the feature/hide_files_in_global_media_library branch 2 times, most recently from 8ea731a to d24e1ef Compare February 9, 2023 13:09
Copy link
Member

@MizukiTemma MizukiTemma left a comment

Choose a reason for hiding this comment

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

🎉

Copy link
Member

@timobrembeck timobrembeck left a comment

Choose a reason for hiding this comment

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

Thanks a lot! 👍

It is going in the right direction, but a few things need to be improved in my opinion.

@JoeyStk JoeyStk force-pushed the feature/hide_files_in_global_media_library branch 4 times, most recently from 889acbc to fcd472a Compare February 14, 2023 14:21
@JoeyStk
Copy link
Contributor Author

JoeyStk commented Feb 14, 2023

Thank you a lot for your suggestions. I tried to work them all in. Please feel free to test it again :)

Copy link
Member

@timobrembeck timobrembeck 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 your changes! 👍
I still have questions regarding the hiding mechanism, what do you think about filtering the directory content already on the server side?

@JoeyStk JoeyStk force-pushed the feature/hide_files_in_global_media_library branch 2 times, most recently from a6d8268 to 7342c6e Compare February 15, 2023 17:37
integreat_cms/cms/views/media/media_actions.py Outdated Show resolved Hide resolved
integreat_cms/cms/views/media/media_actions.py Outdated Show resolved Hide resolved
integreat_cms/cms/views/media/media_actions.py Outdated Show resolved Hide resolved
@JoeyStk JoeyStk force-pushed the feature/hide_files_in_global_media_library branch from 99a02ac to 4e29fe4 Compare February 15, 2023 18:01
integreat_cms/cms/models/media/directory.py Outdated Show resolved Hide resolved
integreat_cms/cms/models/media/media_file.py Outdated Show resolved Hide resolved
@JoeyStk JoeyStk force-pushed the feature/hide_files_in_global_media_library branch from 4e29fe4 to f2fd13a Compare February 15, 2023 18:36
@JoeyStk JoeyStk force-pushed the feature/hide_files_in_global_media_library branch 3 times, most recently from de1dcb2 to 6910ad6 Compare February 18, 2023 09:04
@JoeyStk
Copy link
Contributor Author

JoeyStk commented Feb 18, 2023

Thank you a lot for your suggestions. They are worked in now and you can check again :)

@JoeyStk JoeyStk force-pushed the feature/hide_files_in_global_media_library branch from 6910ad6 to 7300c9f Compare February 18, 2023 15:54
@JoeyStk JoeyStk merged commit 3d18cdc into develop Feb 18, 2023
@JoeyStk JoeyStk deleted the feature/hide_files_in_global_media_library branch February 18, 2023 16:11
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.

Select region icons from region media library instead of global media library
3 participants