element-hq / element-meta

Shared/meta documentation and project artefacts for Element clients
66 stars 11 forks source link

[Story] Improve DMs #2254

Closed VolkerJunginger closed 5 months ago

VolkerJunginger commented 6 months ago

Story

This story lists task to improve the UX for DMs

  1. Remove the following state events at the start of the timeline:

    • "This is the beginning of [username]"
    • You created the room
    • You joined the room
  2. Show the following state events

    • Encryption enabled
    • You invited [username]
    • [username] accepted the invite
  3. In the room details create a new string for "Leave room".

    • A DMs should not be reffered to as a room but a conversation.
    • Change action to "Leave conversation"

Leads

Dependencies

Sign-offs

Android

iOS

Scope

### Android
- [ ] https://github.com/element-hq/element-x-android/issues/2216
- [ ] https://github.com/element-hq/element-x-android/issues/2217
- [ ] https://github.com/element-hq/element-x-android/issues/2218
### iOS
- [ ] https://github.com/element-hq/element-x-ios/issues/2329
- [ ] https://github.com/element-hq/element-x-ios/issues/2330
### Rust
- [ ] https://github.com/matrix-org/matrix-rust-sdk/pull/3025

Out of scope

We decided that the work on read receipts will be done in a broader epic to get them right. The following tasks will then happen later:

They have a dependency on the timeline cache we are going to do this quarter.

jmartinesp commented 6 months ago

About removing the 'this is the beginning of someone', there is also a version with 'this is the beginning of this conversation' that we could use for DMs if we wanted to. I'm just mentioning it in case this wasn't known.

Also, just to confirm: on Android we're displaying way more state events than those, including some placeholders for power levels, room join rules, etc., which we'll probably want to remove (I think iOS is already hiding them). Do we want to hide any events of this kind from the timeline, not only the initial ones?

jmartinesp commented 6 months ago

Also, should removing the initial:

"This is the beginning of [username]" You created the room You joined the room

Be done to all rooms, or just DMs? @VolkerJunginger

VolkerJunginger commented 6 months ago

About removing the 'this is the beginning of someone', there is also a version with 'this is the beginning of this conversation' that we could use for DMs if we wanted to. I'm just mentioning it in case this wasn't known. Thanks, but I think this intro is not needed.

Also, just to confirm: on Android we're displaying way more state events than those, including some placeholders for power levels, room join rules, etc., which we'll probably want to remove (I think iOS is already hiding them). Do we want to hide any events of this kind from the timeline, not only the initial ones?

Yes, please! Thanks for pointing that out.

VolkerJunginger commented 6 months ago

Also, should removing the initial:

"This is the beginning of [username]" You created the room You joined the room

Be done to all rooms, or just DMs? @VolkerJunginger

no, for now just DMs.

jmartinesp commented 6 months ago

@VolkerJunginger I just realised we have this confirmation dialog too when leaving a room: image

Should we also have an alternate message changing room to conversation here?

VolkerJunginger commented 6 months ago

Is this true? If I leave a DM I cannot rejoin?

jmartinesp commented 6 months ago

Is this true? If I leave a DM I cannot rejoin?

Not until you're invited again by the other user, trying to open a DM with them would create a new one.

But that wasn't what my question was about: we have several messages when leaving a room/DM and they all mention 'room' and not 'conversation'. Should we create alternative texts for all those?

When a room is private:

Are you sure that you want to leave this room? This room is not public and you won\'t be able to rejoin without an invite.

When a room will be empty after you leave:

Are you sure that you want to leave this room? You\'re the only person here. If you leave, no one will be able to join in the future, including you.

The generic one:

Are you sure that you want to leave the room?

VolkerJunginger commented 6 months ago

ok, that helps. Then we need an alternative for DMs. Given the info above I would go with this:

| "Are you sure to leave this conversation? You will only be able to rejoin when being invited by [username]"

Would that be possible?

jmartinesp commented 6 months ago

Sure, I just wanted to confirm that's what we wanted to do. I'll create alternative texts for all those in localazy and add to the tasks to do to use these new strings for DMs.

jmartinesp commented 6 months ago

@VolkerJunginger sorry, another question. We want to remove the 'You created the room' message, but does it mean for both the user that created the DM and the invited user? So the invited user wouldn't see 'Someone created the room' either? I think it's fine since the user will have to accept the invite and they should know they didn't create the room, but just double checking.

VolkerJunginger commented 6 months ago

No need to be sorry. You have way more info looking at the code that I do just looking at the apps - I appreciate you bringing these questions up.

Yes, remove for both users.

Velin92 commented 5 months ago

@VolkerJunginger Opened up these two issues for having RRs only displayed on messages and not on state events, if you feel they should be included in a separate story/issue that needs to be specced out here they are: https://github.com/element-hq/element-x-android/issues/2311 https://github.com/matrix-org/matrix-rust-sdk/issues/3065

jmartinesp commented 5 months ago

https://github.com/matrix-org/matrix-rust-sdk/pull/3025 is now merged, but it needs to be implemented in the clients yet.

Be aware the API used for fullRoom() now requires you to first initialize the timeline:

if (!roomListItem.isTimelineInitialized()) {
    roomListItem.initTimeline(eventFilters) // or null/nil if you don't want to pass any filter
}
roomListItem.fullRoom()

The filters I've added myself are:

val eventFilters = TimelineEventTypeFilter.exclude(
    listOf(
        FilterStateEventType.ROOM_ALIASES,
        FilterStateEventType.ROOM_CANONICAL_ALIAS,
        FilterStateEventType.ROOM_GUEST_ACCESS,
        FilterStateEventType.ROOM_HISTORY_VISIBILITY,
        FilterStateEventType.ROOM_JOIN_RULES,
        FilterStateEventType.ROOM_PINNED_EVENTS,
        FilterStateEventType.ROOM_POWER_LEVELS,
        FilterStateEventType.ROOM_SERVER_ACL,
        FilterStateEventType.ROOM_TOMBSTONE,
        FilterStateEventType.SPACE_CHILD,
        FilterStateEventType.SPACE_PARENT,
        FilterStateEventType.POLICY_RULE_ROOM,
        FilterStateEventType.POLICY_RULE_SERVER,
        FilterStateEventType.POLICY_RULE_USER,
    ).map(FilterTimelineEventType::State)
)
Velin92 commented 5 months ago

matrix-org/matrix-rust-sdk#3025 is now merged, but it needs to be implemented in the clients yet.

Be aware the API used for fullRoom() now requires you to first initialize the timeline:

if (!roomListItem.isTimelineInitialized()) {
    roomListItem.initTimeline(eventFilters) // or null/nil if you don't want to pass any filter
}
roomListItem.fullRoom()

The filters I've added myself are:

val eventFilters = TimelineEventTypeFilter.exclude(
    listOf(
        FilterStateEventType.ROOM_ALIASES,
        FilterStateEventType.ROOM_CANONICAL_ALIAS,
        FilterStateEventType.ROOM_GUEST_ACCESS,
        FilterStateEventType.ROOM_HISTORY_VISIBILITY,
        FilterStateEventType.ROOM_JOIN_RULES,
        FilterStateEventType.ROOM_PINNED_EVENTS,
        FilterStateEventType.ROOM_POWER_LEVELS,
        FilterStateEventType.ROOM_SERVER_ACL,
        FilterStateEventType.ROOM_TOMBSTONE,
        FilterStateEventType.SPACE_CHILD,
        FilterStateEventType.SPACE_PARENT,
        FilterStateEventType.POLICY_RULE_ROOM,
        FilterStateEventType.POLICY_RULE_SERVER,
        FilterStateEventType.POLICY_RULE_USER,
    ).map(FilterTimelineEventType::State)
)

Just opened a PR for it on El-X-iOS: https://github.com/element-hq/element-x-ios/pull/2404

VolkerJunginger commented 5 months ago

Please also update the localazy translations.