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

[Story] Improve DMs #2254

Closed
6 tasks done
VolkerJunginger opened this issue Jan 5, 2024 · 15 comments
Closed
6 tasks done

[Story] Improve DMs #2254

VolkerJunginger opened this issue Jan 5, 2024 · 15 comments
Labels
App: ElementX Android App: ElementX iOS T-Epic Issue is at Epic level T-User Story X-Needs-Signoff Stories and Epics which are ready for review by product, design and QA

Comments

@VolkerJunginger
Copy link
Contributor

VolkerJunginger commented Jan 5, 2024

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
  1. Show the following state events
  • Encryption enabled
  • You invited [username]
  • [username] accepted the invite
  1. 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

  • TBD

Sign-offs

Android

  • Design sign-off on completion
  • QA sign-off on completion
  • Product sign-off on completion

iOS

  • Design sign-off on completion
  • QA sign-off on completion
  • Product sign-off on completion

Scope

Android

Preview Give feedback
  1. T-Task
    jmartinesp
  2. T-Task
    jmartinesp
  3. T-Task
    jmartinesp

iOS

Preview Give feedback
  1. T-Task
  2. T-Task
    Velin92

Rust

Preview Give feedback

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:
- element-hq/element-x-android#2311
- matrix-org/matrix-rust-sdk#3065

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

@jmartinesp
Copy link
Member

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 jmartinesp self-assigned this Jan 12, 2024
@jmartinesp
Copy link
Member

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
Copy link
Contributor Author

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
Copy link
Contributor Author

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.

@manuroe manuroe added T-Epic Issue is at Epic level and removed X-Needs-Leads Needs design and tech representatives labels Jan 12, 2024
@jmartinesp
Copy link
Member

@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
Copy link
Contributor Author

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

@jmartinesp
Copy link
Member

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
Copy link
Contributor Author

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
Copy link
Member

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
Copy link
Member

@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
Copy link
Contributor Author

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
Copy link
Member

Velin92 commented Jan 29, 2024

@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:
element-hq/element-x-android#2311
matrix-org/matrix-rust-sdk#3065

@jmartinesp
Copy link
Member

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)
)

@Velin92
Copy link
Member

Velin92 commented Jan 30, 2024

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:
element-hq/element-x-ios#2404

@manuroe manuroe added the X-Needs-Signoff Stories and Epics which are ready for review by product, design and QA label Feb 1, 2024
@manuroe manuroe added X-Needs-Signoff Stories and Epics which are ready for review by product, design and QA and removed X-Needs-Signoff Stories and Epics which are ready for review by product, design and QA labels Feb 1, 2024
@VolkerJunginger
Copy link
Contributor Author

Please also update the localazy translations.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
App: ElementX Android App: ElementX iOS T-Epic Issue is at Epic level T-User Story X-Needs-Signoff Stories and Epics which are ready for review by product, design and QA
Projects
None yet
Development

No branches or pull requests

4 participants