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

✨ #391 - Allow description to be shown by a file. #207

Merged
merged 22 commits into from
May 30, 2022

Conversation

svenvandescheur
Copy link
Contributor

Schermafbeelding 2022-05-03 om 13 58 11

Can be populated by filling in the description field on the filer file.

@svenvandescheur svenvandescheur requested a review from JostCrow May 3, 2022 11:59
@svenvandescheur
Copy link
Contributor Author

taiga-oip-453

@svenvandescheur
Copy link
Contributor Author

Schermafbeelding 2022-05-03 om 14 34 56

@svenvandescheur
Copy link
Contributor Author

Schermafbeelding 2022-05-03 om 14 41 52

@svenvandescheur
Copy link
Contributor Author

taiga-oip-534

@svenvandescheur
Copy link
Contributor Author

Schermafbeelding 2022-05-03 om 15 09 45

@svenvandescheur
Copy link
Contributor Author

Schermafbeelding 2022-05-03 om 16 40 52

@svenvandescheur
Copy link
Contributor Author

Schermafbeelding 2022-05-03 om 17 42 29

@svenvandescheur
Copy link
Contributor Author

taiga-oip-482

@@ -134,6 +134,9 @@ def get(self, request, *args, **kwargs):
"""Mark all messages as seen for the receiver"""
context = self.get_context_data()

if not request.GET.get("redirected"):
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe add a comment why this is added. This is not clear to me

This comment was marked as resolved.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

To move to then end of the conversation

Copy link
Contributor

Choose a reason for hiding this comment

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

That would be a usefull comment. So that we know why the redirect should stay there

Copy link
Contributor

@JostCrow JostCrow left a comment

Choose a reason for hiding this comment

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

Still need to go over the changes visually

@@ -134,6 +134,9 @@ def get(self, request, *args, **kwargs):
"""Mark all messages as seen for the receiver"""
context = self.get_context_data()

if not request.GET.get("redirected"):

This comment was marked as resolved.

Copy link
Contributor

@JostCrow JostCrow left a comment

Choose a reason for hiding this comment

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

This is not approved yet. Since 2 issues are not solved for now.
#445 is not completely solved yet.
#481 is not solved.

@svenvandescheur
Copy link
Contributor Author

I think #481 should be discussed in #213

@alextreme
Copy link
Member

Taiga 445 is something Anna is working on and can be done separately
Taiga 481 is indeed picked up in PR #213

@JostCrow and @annashamray please discuss this PR and if it needs further work. After the Actions Anna can continue with the Messages

@JostCrow
Copy link
Contributor

One of my issues is still open of the missing comment. Since that will help with maintenance later on and the conflict that needs to be fixed. If the 2 open issues are solved in other pull requests that will be fine and that comment can be disregarded.

@alextreme
Copy link
Member

I noticed that a comment was already added when viewing the PR. Sven is on a holiday this week, this is the reason I've gotten involved.

@JostCrow
Copy link
Contributor

Yes, but the comment is not clear enough. Because I did not know how it is now in the code that is is supposed to scroll down to the latest message in the conversation

@vaszig vaszig requested a review from alextreme May 25, 2022 14:12
@alextreme alextreme removed their request for review May 25, 2022 21:15
@alextreme
Copy link
Member

PR is for Jorik to approve/decline imho

@alextreme alextreme merged commit 34bdf2c into develop May 30, 2022
@alextreme alextreme deleted the issues#2022-05-03 branch May 30, 2022 13:21
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.

3 participants