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

MSC4052: Hiding read receipts UI in certain rooms #4052

Closed
wants to merge 3 commits into from

Conversation

cloudrac3r
Copy link

Rendered

Signed-off-by: Cadence Ember cadence@disroot.org

@cloudrac3r cloudrac3r changed the title MSC4052: Hiding read receipts UI in certain bridged rooms MSC4052: Hiding read receipts UI in certain rooms Sep 6, 2023
@turt2live turt2live added proposal A matrix spec change proposal client-server Client-Server API kind:feature MSC for not-core and not-maintenance stuff needs-implementation This MSC does not have a qualifying implementation for the SCT to review. The MSC cannot enter FCP. labels Sep 6, 2023

The `m.hide_ui` event is designed to be sent by a compatible appservice bridge when it manages a bridged room. It can also be sent manually for any reason.

In the future, this concept could be extended to hide other parts of the UI using other state keys. Sticking with IRC as an example, the bridge bot may ask clients to hide "add reaction" buttons, "create rich reply" buttons, "upload file" buttons and so on, if it knows that it cannot handle those events in a satisfactory way. The exact details are out of scope for MSC4052. MSC4052 only specifies hiding read receipts in the UI.
Copy link

Choose a reason for hiding this comment

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

This specific use case sounds like an xy problem, better solved by msc3968

Copy link
Author

Choose a reason for hiding this comment

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

Could you please elaborate on how it is an XY problem?

Copy link

Choose a reason for hiding this comment

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

This isn't a msc for discouraging feature usage, it's one written for hiding ui to discourage feature use.

msc3968 takes the other approach, specifying which features are encouraged/discouraged then letting clients prioritize/deprioritize/hide ui elements based on that

Copy link
Author

Choose a reason for hiding this comment

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

Okay, I've been thinking about this a lot and I agree with you. I think MSC3968 is a better direction to take it. However, 3968 doesn't currently provide a way of disabling read receipts, since the author didn't think of that. Is it possible for me to collaborate on 3968 in a more significant way than asking the author to make changes in the comments?

Comment on lines +9 to +13
A new room state event `m.hide_ui` with state key `read_receipts`
determines whether read receipts should be hidden in the UI. To opt-in
to hiding read receipts, the event should contain `{"hidden": true}`.
Other properties are ignored. If `hidden` is not `true` then read
receipts are not hidden.
Copy link
Member

Choose a reason for hiding this comment

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

My drive-by take on this is that this is probably better handled by having a state event, say m.room.features contain a dictionary of supported features in the room; read_receipts being one of them.

A room then created by an IRC bridge would set the read_receipt feature to false, and clients could adjust their UI accordingly.

Copy link
Author

@cloudrac3r cloudrac3r Sep 11, 2023

Choose a reason for hiding this comment

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

by having a state event...contain a dictionary of supported features

I thought about this at the time, but I decided having a separate state key for each feature would work better, to avoid a similar hassle to how difficult it is to update power levels.

For example, to update power levels currently, one has to fetch the current power levels state, make changes to it in memory, and then set that modified dictionary as the state: https://github.com/matrix-org/matrix-appservice-bridge/blob/2334b0bae28a285a767fe7244dad59f5a5963037/src/components/intent.ts#L352 While this works, it is hard to read and prone to bugs.

It's also a similar problem to updating m.direct in account data, which also requires one to fetch the existing event first and modify it so that they don't accidentally erase all of the existing data.

I can bypass that problem by having each event in a separate state key. Then to hide or show a particular feature, I can unconditionally send a state event of {"hidden": true} without being concerned about what was already there.

Copy link
Member

Choose a reason for hiding this comment

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

I see, and that makes sense. I suppose my main point was to eliminate the "UI" wording in the state event. So we could still use individual state keys for each feature, but the type would be m.room.feature (now singular), instead of m.hide_ui.

@cloudrac3r
Copy link
Author

Thank you everybody for the feedback. I will try to collaborate on #3968 to get read receipts in there instead.

@cloudrac3r cloudrac3r closed this Oct 5, 2023
@turt2live turt2live added the obsolete A proposal which has been overtaken by other proposals label Oct 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
client-server Client-Server API kind:feature MSC for not-core and not-maintenance stuff needs-implementation This MSC does not have a qualifying implementation for the SCT to review. The MSC cannot enter FCP. obsolete A proposal which has been overtaken by other proposals proposal A matrix spec change proposal
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants