Skip to content
This repository has been archived by the owner on Sep 11, 2024. It is now read-only.

fix automatic DM avatar with functional members #12157

Merged

Conversation

HarHarLinks
Copy link
Contributor

@HarHarLinks HarHarLinks commented Jan 19, 2024

"Functional Members" is an Element feature that allows you to mark certain room members as bots or similar, allowing clients to exclude these members from things like autogenerated room names and avatars or could even be used to show that these members are not "natural person" users. The feature is described at https://github.com/element-hq/element-meta/blob/develop/spec/functional_members.md.

Element Web through the js and react SDKs already implements some of this feature, namely the exclusion from functional members from autogenerated room names. This PR expands the implementation for the avatar and presence dot.

before:
image

after:
image

needs: matrix-org/matrix-js-sdk#4017

Checklist

  • Tests written for new code (and old code if feasible)
  • Linter and other CI checks pass
  • Sign-off given on the changes (see CONTRIBUTING.md)

Signed-off-by: Kim Brose 2803622+HarHarLinks@users.noreply.github.com


This PR currently has none of the required changelog labels.

A reviewer can add one of: T-Deprecation, T-Enhancement, T-Defect, T-Task to indicate what type of change this is, or add Type: [enhancement/defect/task] to the description and I'll add them for you.

@HarHarLinks

This comment was marked as outdated.

@HarHarLinks HarHarLinks force-pushed the fix-functional-members-avatar branch from 625760a to a88a576 Compare February 16, 2024 16:47
@richvdh
Copy link
Member

richvdh commented Feb 19, 2024

@HarHarLinks: sorry, I've not forgotten about this. I want to discuss it with the team a bit, though.

@HarHarLinks
Copy link
Contributor Author

HarHarLinks commented Feb 19, 2024

Thank you for the feedback, and based on your reaction I want to say sorry if the review pings were annoyingly sent repeatedly, I did not trigger them manually.

Please don't judge my code too harshly, this is my 3rd time or so messing with js/ts/react and I tried my hardest to figure it out and meet your standards.

I should note that the equivalent of this feature recently landed in Android 1.6.10 element-hq/element-android#8700 per element-hq/element-android#3736 and the first half, the removal of functional members' names from automatically generated room names has been present in web for a long time matrix-org/matrix-js-sdk#1771, so it would make sense to complete the feature, which is what I tried to do here. It is also implemented on iOS element-hq/element-ios#4643.

@richvdh
Copy link
Member

richvdh commented Feb 19, 2024

Thank you for the feedback, and based on your reaction I want to say sorry if the review pings were annoyingly sent repeatedly, I did not trigger them manually.

Not at all. I just noticed this had already been sitting for a couple of weeks.

Copy link
Member

@richvdh richvdh left a comment

Choose a reason for hiding this comment

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

Looks like the snapshot needs updating, and a couple of suggstions for the tests.

@richvdh
Copy link
Member

richvdh commented Feb 23, 2024

Thanks very much for the contributions, by the way!

@HarHarLinks
Copy link
Contributor Author

Thanks for the feedback, all 3 points should now have been addressed.

@richvdh richvdh added the T-Defect Bugs, crashes, hangs, vulnerabilities, or other reported problems label Feb 26, 2024
Copy link
Member

@richvdh richvdh left a comment

Choose a reason for hiding this comment

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

lgtm. Thanks!

@richvdh richvdh enabled auto-merge February 26, 2024 18:10
@richvdh
Copy link
Member

richvdh commented Feb 26, 2024

@HarHarLinks this seems to be the victim of some flaky tests. I've re-run them, but if you notice them blocking this PR again, please ping us.

@richvdh
Copy link
Member

richvdh commented Feb 26, 2024

Still failing :(

cf element-hq/element-web#27075, element-hq/element-web#27076

@richvdh richvdh added this pull request to the merge queue Mar 3, 2024
Merged via the queue into matrix-org:develop with commit 65bfc92 Mar 3, 2024
19 checks passed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
T-Defect Bugs, crashes, hangs, vulnerabilities, or other reported problems Z-Community-PR Issue is solved by a community member's PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants