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

Use styled mxids in member list - #6328

Merged
turt2live merged 18 commits into
matrix-org:developfrom
SimonBrandner:feature/disambiguated-profile/14825
Mar 22, 2022
Merged

Use styled mxids in member list#6328
turt2live merged 18 commits into
matrix-org:developfrom
SimonBrandner:feature/disambiguated-profile/14825

Conversation

@SimonBrandner

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

Copy link
Copy Markdown
Contributor

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

ShadowJonathan commented Jul 7, 2021

Copy link
Copy Markdown

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

t3chguy commented Jul 7, 2021 •
edited
Loading

Copy link
Copy Markdown
Member

Keep in mind below is already used for status/presence

image

@t3chguy

t3chguy commented Jul 7, 2021

Copy link
Copy Markdown
Member

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

ShadowJonathan commented Jul 7, 2021

Copy link
Copy Markdown

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

aaronraimist commented Jul 7, 2021

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

@turt2live turt2live left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

code generally seems fine

Comment thread res/css/_components.scss Outdated
@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

ara4n commented Mar 22, 2022

Copy link
Copy Markdown
Member

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

ara4n commented Mar 22, 2022 •
edited
Loading

Copy link
Copy Markdown
Member

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

@turt2live

turt2live commented Mar 22, 2022

Copy link
Copy Markdown
Member

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

@turt2live

turt2live commented Mar 22, 2022

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

turt2live commented Mar 22, 2022

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

2 more reviewers
@turt2live turt2live turt2live approved these changes
@ara4n ara4n ara4n approved these changes
Reviewers whose approvals may not affect merge requirements

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