-
Notifications
You must be signed in to change notification settings - Fork 232
Reply action: harmonize condition #1271
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
Conversation
add(TimelineItemAction.Reply) | ||
if (userCanSendMessage) { | ||
add(TimelineItemAction.Reply) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is fix 1
@@ -189,7 +190,7 @@ fun TimelineItemRow( | |||
TimelineItemEventRow( | |||
event = timelineItem, | |||
isHighlighted = highlightedItem == timelineItem.identifier(), | |||
canReply = canReply, | |||
canReply = userHasPermissionToSendMessage && timelineItem.content.canBeReplied(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is fix 2
📱 Scan the QR code below to install the build (arm64 only) for this PR. |
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## develop #1271 +/- ##
===========================================
+ Coverage 57.42% 57.66% +0.24%
===========================================
Files 1086 1087 +1
Lines 28270 28570 +300
Branches 5806 5857 +51
===========================================
+ Hits 16234 16475 +241
- Misses 9495 9525 +30
- Partials 2541 2570 +29
☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, a minor comment.
@@ -101,7 +103,8 @@ class ActionListPresenter @Inject constructor( | |||
buildList { | |||
val isMineOrCanRedact = timelineItem.isMine || userCanRedact | |||
|
|||
// TODO Poll: Reply to poll | |||
// TODO Poll: Reply to poll. Ensure to update `fun TimelineItemEventContent.canBeReplied()` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👌
* Determine if the event content can be replied to. | ||
* Note: it should match the logic in [io.element.android.features.messages.impl.actionlist.ActionListPresenter]. | ||
*/ | ||
fun TimelineItemEventContent.canBeReplied(): Boolean = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: it should probably be canBeRepliedTo
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Kudos, SonarCloud Quality Gate passed! |
Type of change
Content
Harmonize condition to enable or disable reply action.
Motivation and context
Closes #1173
Screenshots / GIFs
Tests
Tested devices
Checklist