Skip to content

Communication: Resolve Post functionality for threads #59

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

Merged
merged 32 commits into from
Nov 14, 2024

Conversation

julian-wls
Copy link
Contributor

Description:

This PR includes the following features and fixes:

  • A new PostAction in the PostActionContextBottomSheet that allows users to mark AnswerPosts in threads as resolving posts (Fig. 1)
  • A new label on post items indicating if it is a resolved post or if it resolves a parent post (Fig. 2)
  • Three minor fixes:
    • Loads text into the markdown text field when editing post button has been clicked
    • Removes line limit of post items to allow longer messages to resolve Messages are not shown entirely #52
    • Requests reload after successful completion of post modifications in the conversation view model to apply changes to the view

Screenshots:

Fig. 1:
Screenshot_20241019_103154

Fig. 2:
Screenshot_20241019_103212

If the post already resolves the parent posts users can remove this tag:
Screenshot_20241019_103223

@julian-wls julian-wls self-assigned this Oct 19, 2024
@julian-wls julian-wls requested a review from TimOrtel October 20, 2024 07:58
@ahbitaqu
Copy link

ahbitaqu commented Oct 20, 2024

Looks great so far! Just two minor issues:

  1. Even though I can see if the post has been resolved by someone, resolving the post through the app doesn't work for me
  2. When you react with an emoji, the view is not updated, so you have to open the thread again to see even your own reaction.

Thanks for working on this :)

@ahbitaqu
Copy link

Also, is there any way we could make the resolved tags colored in green? might improve the visual overview drastically

@julian-wls
Copy link
Contributor Author

Even though I can see if the post has been resolved by someone, resolving the post through the app doesn't work for me
When you react with an emoji, the view is not updated, so you have to open the thread again to see even your own reaction.

That's correct, but as described in the discussion above this problem is unrelated to this pr and is part of issue #60

Also, is there any way we could make the resolved tags colored in green? might improve the visual overview drastically

Sure, that’s something I had in mind, too. However I think this should happen in form of another pr that improves the overall design of post items to align them with iOS. I don’t think it fits the current state of the design.

Copy link

@ahbitaqu ahbitaqu 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 the info! Good to go from my side

@ahbitaqu
Copy link

ahbitaqu commented Oct 20, 2024

That's correct, but as described in the discussion above this problem is unrelated to this pr and is part of issue #60

Just that we're on the same page: the resolve button doesn't work at all. It's not just the UI, the information is never reflected on the server even through the web view. As I understand #60 deals with local representation only

@julian-wls
Copy link
Contributor Author

julian-wls commented Oct 20, 2024

Thanks for pointing that out. Just checked it, and I somehow forgot to invoke the function call in my previous commit. Sorry for the confusion. Should work now.

@julian-wls julian-wls requested a review from ahbitaqu October 20, 2024 17:05
Copy link

@ahbitaqu ahbitaqu left a comment

Choose a reason for hiding this comment

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

Works now, Thank you! :)

@julian-wls julian-wls requested a review from TimOrtel October 21, 2024 08:49
Copy link
Contributor

@TimOrtel TimOrtel left a comment

Choose a reason for hiding this comment

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

Please check your junit tests. They seem to be failing in the ci

@FelberMartin FelberMartin changed the base branch from main to develop October 25, 2024 13:51
@julian-wls julian-wls requested a review from TimOrtel October 29, 2024 13:34
@FelberMartin FelberMartin linked an issue Nov 9, 2024 that may be closed by this pull request
@julian-wls julian-wls requested a review from TimOrtel November 9, 2024 14:24
Copy link
Collaborator

@FelberMartin FelberMartin left a comment

Choose a reason for hiding this comment

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

Looks great overall, looking forward to merge this as soon as possible. Just one small fix :)

@julian-wls julian-wls added the ready to merge This PR can be merged label Nov 13, 2024
@FelberMartin FelberMartin merged commit 1d9afac into develop Nov 14, 2024
3 of 4 checks passed
@FelberMartin FelberMartin deleted the feature/communication/resolve-threads branch November 14, 2024 08:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready to merge This PR can be merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Communication: Improve Edit post flow Messages are not shown entirely
4 participants