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

Use styled mxids in member list #6328

Merged
merged 18 commits into from
Mar 22, 2022
Merged

Use styled mxids in member list #6328

merged 18 commits into from
Mar 22, 2022

Conversation

SimonBrandner
Copy link
Contributor

@SimonBrandner SimonBrandner commented Jul 7, 2021 •
edited by github-actions bot
Loading

Fixes element-hq/element-web#14825

Screenshot_20210707_182644

Preview: https://pr6328--matrix-react-sdk.netlify.app
⚠️ Do you trust the author of this PR? Maybe this build will steal your keys or give you malware. Exercise caution. Use test accounts.

@SimonBrandner
Add DisambiguatedProfile
c08600d 
Signed-off-by: Šimon Brandner <simon.bra.ag@gmail.com>
@SimonBrandner
Give DisambiguatedProfile some nice options
d00f095 
Signed-off-by: Šimon Brandner <simon.bra.ag@gmail.com>
@SimonBrandner
Delint
d26c82c 
Signed-off-by: Šimon Brandner <simon.bra.ag@gmail.com>
@SimonBrandner
Use DisambiguatedProfile in member tile
d726963 
Signed-off-by: Šimon Brandner <simon.bra.ag@gmail.com>
@SimonBrandner
Basic handling of text overflow
f002ec1 
Signed-off-by: Šimon Brandner <simon.bra.ag@gmail.com>
@SimonBrandner
Use name instead of rawDisplayName
f86828d 
Signed-off-by: Šimon Brandner <simon.bra.ag@gmail.com>
@SimonBrandner
This seems to make more sense
fc3a68e 
Signed-off-by: Šimon Brandner <simon.bra.ag@gmail.com>
@SimonBrandner SimonBrandner marked this pull request as ready for review July 7, 2021 16:31
@ShadowJonathan ShadowJonathan mentioned this pull request Jul 7, 2021
Closed
@ShadowJonathan
Copy link

Wouldn't it style better if the disambiguated username goes below the name displayed there?

Else it'll be cut off if the displayname is too long.

(For that matter, wouldn't it make sense to always show usernames under displaynames in the sidebar? I think it warrants to always display info like that in a sidebar)

@t3chguy
Copy link
Member

t3chguy commented Jul 7, 2021 •
edited
Loading

Keep in mind below is already used for status/presence

image

@t3chguy
Copy link
Member

t3chguy commented Jul 7, 2021

For that matter, wouldn't it make sense to always show usernames under displaynames in the sidebar?

No, hiding MXIDs in as many places as is sane is very much intentional.

@ShadowJonathan
Copy link

Good point, though i still think it cutting off if the displayname is too long could be an annoyance.

@t3chguy t3chguy requested review from a team July 7, 2021 18:03
@aaronraimist
Copy link
Contributor

They already get cut off in the current version. At least the font size is a bit smaller here so it will require more characters to be cut off. If the user doesn't want it to cut off the usernames they can expand the right panel.

@SimonBrandner
Merge remote-tracking branch 'upstream/develop' into feature/disambig…
0c181ae 
…uated-profile/14825

Signed-off-by: Šimon Brandner <simon.bra.ag@gmail.com>
@SimonBrandner
Reodred
722316c 
Signed-off-by: Šimon Brandner <simon.bra.ag@gmail.com>
turt2live
turt2live approved these changes Jul 22, 2021
View reviewed changes
Copy link
Member

@turt2live turt2live left a comment

Choose a reason for hiding this comment

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

code generally seems fine

res/css/_components.scss Outdated Show resolved Hide resolved
@SimonBrandner
rethemedex
a2883cf 
Signed-off-by: Šimon Brandner <simon.bra.ag@gmail.com>
@SimonBrandner
Merge remote-tracking branch 'upstream/develop' into feature/disambig…
4f97158 
…uated-profile/14825
@nadonomy nadonomy self-assigned this Oct 19, 2021
@SimonBrandner
Merge remote-tracking branch 'upstream/develop' into feature/disambig…
43e5ec0 
…uated-profile/14825

Signed-off-by: Šimon Brandner <simon.bra.ag@gmail.com>
@SimonBrandner
Merge remote-tracking branch 'upstream/develop' into feature/disambig…
96a4b4c 
…uated-profile/14825
@SimonBrandner
Fix import
f7a0ac8 
Signed-off-by: Šimon Brandner <simon.bra.ag@gmail.com>
@SimonBrandner
Fix color
60453e0 
Signed-off-by: Šimon Brandner <simon.bra.ag@gmail.com>
@ara4n
Copy link
Member

ara4n commented Mar 22, 2022

This has been blocked on review by design for 9 months (cc @nadonomy), and it's a concrete useful security feature (unspoofable disambiguation) which from the screenshots is a clear cosmetic improvement. So I'm making an exec decision to approve it. @SimonBrandner - thanks for doing it, and sorry it got stuck for so long.

@ara4n
Copy link
Member

ara4n commented Mar 22, 2022 •
edited
Loading

(have tried to fix the merge conflict from within GH issue UI 'blind', hopefully it worked... :S)

@turt2live
Copy link
Member

Looks like it's failing modern code checks - will take a look shortly.

@turt2live
Copy link
Member

ftr, this missed #7595 but have re-applied it. From looking at the merge diff, it's the only feature at risk.

Should be safe to include this in the RC tomorrow. If not, it's an easy revert and something we can re-apply onto develop.

@turt2live turt2live enabled auto-merge (squash) March 22, 2022 01:06
@turt2live turt2live merged commit 5d28e05 into matrix-org:develop Mar 22, 2022
turt2live added a commit that referenced this pull request Mar 22, 2022
@turt2live
Revert "Use styled mxids in member list (#6328)"
17cceff 
This reverts commit 5d28e05.
@turt2live turt2live mentioned this pull request Mar 22, 2022
Merged
turt2live added a commit that referenced this pull request Mar 22, 2022
@turt2live
Revert "Use styled mxids in member list (#6328)" (#8107)
709e6e7 
This reverts commit 5d28e05.
@turt2live
Copy link
Member

this ended up getting reverted - see #8107 for details

@SimonBrandner SimonBrandner deleted the feature/disambiguated-profile/14825 branch March 22, 2022 05:23
SimonBrandner added a commit to SimonBrandner/matrix-react-sdk that referenced this pull request Mar 22, 2022
@SimonBrandner
Revert "Revert "Use styled mxids in member list (matrix-org#6328)" (m…
bf6406c 
…atrix-org#8107)"

This reverts commit 709e6e7.
turt2live pushed a commit that referenced this pull request Mar 22, 2022
@SimonBrandner
Use styled mxids in member list v2 (#8110)
bff1ef3 
* Revert "Revert "Use styled mxids in member list (#6328)" (#8107)"

This reverts commit 709e6e7.

* Fix disambiguated profile for bubbles

Signed-off-by: Šimon Brandner <simon.bra.ag@gmail.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@turt2live turt2live turt2live approved these changes

@ara4n ara4n ara4n approved these changes

Assignees

@nadonomy nadonomy

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Appending mxids to display names is strange
7 participants
@SimonBrandner @ShadowJonathan @t3chguy @aaronraimist @ara4n @turt2live @nadonomy