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.

Sign up for GitHub

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

Jump to bottom

Masquer les identifiants matrix #8

Merged
merged 7 commits into from
Sep 4, 2024
Merged

Masquer les identifiants matrix #8

merged 7 commits into from
Sep 4, 2024

Conversation

yostyle
Copy link

@yostyle yostyle commented Aug 21, 2024 •
edited
Loading

Content

Motivation and context

#4

Screenshots / GIFs

Tests

  • Step 1
  • Step 2
  • Step ...

Tested devices

  • Physical
  • Emulator
  • OS version(s):

Checklist

  • Changes have been tested on an Android device or Android emulator with API 23
  • UI change has been tested on both light and dark themes
  • Accessibility has been taken into account. See https://github.com/element-hq/element-x-android/blob/develop/CONTRIBUTING.md#accessibility
  • Pull request is based on the develop branch
  • Pull request title will be used in the release note, it clearly define what will change for the user
  • Pull request includes screenshots or videos if containing UI changes
  • Pull request includes a sign off
  • You've made a self review of your PR

@yostyle yostyle marked this pull request as ready for review August 21, 2024 10:12
@yostyle yostyle requested a review from NicolasBuquet August 21, 2024 10:12
NicolasBuquet
NicolasBuquet reviewed Aug 21, 2024
View reviewed changes
Copy link

@NicolasBuquet NicolasBuquet left a comment

Choose a reason for hiding this comment

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

Pleins de commentaires après ne relecture studieuse :)

...ile/shared/src/main/kotlin/io/element/android/features/userprofile/shared/UserProfileView.kt Outdated
@@ -69,7 +70,7 @@ fun UserProfileView(
UserProfileHeaderSection(
avatarUrl = state.avatarUrl,
userId = state.userId,
userName = state.userName,
userName = state.userName ?: state.userId.toDisplayName(), // TCHAP hide the Matrix Id
openAvatarPreview = { avatarUrl ->
openAvatarPreview(state.userName ?: state.userId.value, avatarUrl)
Copy link

Choose a reason for hiding this comment

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

Ne faudrait-il pas faire la même modification que 2 lignes plus haut ?

libraries/tchaputils/src/main/java/fr/gouv/tchap/libraries/tchaputils/TchapPatterns.kt Outdated
* @param displayName the display name to compute.
* @return displayName without domain (or the display name itself if no domain has been found).
*/
fun getNameFromDisplayName(displayName: String): String {
Copy link

Choose a reason for hiding this comment

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

Utiliser une regex ?

Copy link
Author

Choose a reason for hiding this comment

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

J'ai refactorisé

libraries/tchaputils/src/main/java/fr/gouv/tchap/libraries/tchaputils/TchapPatterns.kt Outdated
* @return displayName without name, empty string if no domain is available.
*/
fun getDomainFromDisplayName(displayName: String): String {
return displayName.split(DISPLAY_NAME_FIRST_DELIMITER)
Copy link

Choose a reason for hiding this comment

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

Utiliser ne regex ?

Copy link
Author

Choose a reason for hiding this comment

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

J'ai refactorisé

...l/src/main/kotlin/io/element/android/libraries/textcomposer/model/MarkdownTextEditorState.kt Outdated
@@ -72,7 +72,7 @@ class MarkdownTextEditorState(
}
is ResolvedMentionSuggestion.Member -> {
val currentText = SpannableStringBuilder(text.value())
val text = mention.roomMember.displayName?.prependIndent("@") ?: mention.roomMember.userId.value
val text = mention.roomMember.displayName?.prependIndent("@") ?: mention.roomMember.userId.value // TCHAP check needed about mxid displaying
Copy link

Choose a reason for hiding this comment

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

Changer le commentaire en // TCHAP TODO ?

Copy link
Author

Choose a reason for hiding this comment

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

je me pose encore la question

@yostyle yostyle force-pushed the yostyle/user_displayname branch from 12aa14e to 73674a0 Compare August 29, 2024 15:11
@yostyle yostyle requested a review from NicolasBuquet August 29, 2024 15:17
@yostyle yostyle added the PR-Change For updates to an existing feature label Aug 29, 2024
@yostyle yostyle self-assigned this Aug 29, 2024
@yostyle yostyle changed the title Hide user ids Masquer les identifiants matrix Aug 29, 2024
NicolasBuquet
NicolasBuquet approved these changes Aug 29, 2024
View reviewed changes
Copy link

@NicolasBuquet NicolasBuquet left a comment

Choose a reason for hiding this comment

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

LGTM!

PS : tu as compté le nb de 'isDebugBuild' ? 😆

@yostyle yostyle force-pushed the yostyle/user_displayname branch from d9dfc66 to 10012d4 Compare September 4, 2024 16:40
@yostyle yostyle force-pushed the yostyle/user_displayname branch from 10012d4 to 68ca992 Compare September 4, 2024 16:48
@yostyle yostyle added the Record-Screenshots Runs the 'Record Screenshots' CI job and adds a commit with any new screenshots found. label Sep 4, 2024
@github-actions github-actions bot removed the Record-Screenshots Runs the 'Record Screenshots' CI job and adds a commit with any new screenshots found. label Sep 4, 2024
@yostyle yostyle merged this pull request into develop Sep 4, 2024
yostyle added a commit that referenced this pull request Sep 5, 2024
@yostyle
Masquer les identifiants matrix (#8)
56b048e
yostyle added a commit that referenced this pull request Oct 7, 2024
@yostyle
Masquer les identifiants matrix (#8)
4546553
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@NicolasBuquet NicolasBuquet NicolasBuquet approved these changes

Assignees

@yostyle yostyle

Labels
PR-Change For updates to an existing feature
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@yostyle @NicolasBuquet