Add missing device id to settings screen #2320
Conversation
|
📱 Scan the QR code below to install the build (arm64 only) for this PR. |
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## develop #2320 +/- ##
========================================
Coverage 70.05% 70.06%
========================================
Files 1353 1353
Lines 33233 33249 +16
Branches 6872 6876 +4
========================================
+ Hits 23283 23295 +12
- Misses 6641 6642 +1
- Partials 3309 3312 +3 ☔ View full report in Codecov by Sentry. |
| @@ -62,6 +65,10 @@ class PreferencesRootPresenter @Inject constructor( | |||
| initialLoad(matrixUser) | |||
| } | |||
|
|
|||
| val deviceId by produceState<String?>(initialValue = null) { | |||
| value = sessionStore.getSession(matrixClient.sessionId.value)?.deviceId | |||
There was a problem hiding this comment.
I would probably expose the deviceId from the MatrixClient, as a String, this would be simpler.
We could use |
I can give it a try, but I remember it causing issues in the past so we stopped using it. Maybe those have been fixed in latest Paparazzi versions. |
jmartinesp
added
the
Record-Screenshots
github-actions
bot
removed
the
Record-Screenshots
|
It's actually using |
| @@ -64,6 +64,7 @@ class RustMatrixClientFactory @Inject constructor( | |||
| baseDirectory = baseDirectory, | |||
| baseCacheDirectory = cacheDirectory, | |||
| clock = clock, | |||
| deviceId = sessionData.deviceId, | |||
There was a problem hiding this comment.
I would not add a constructor param here, but read the value from client.deviceId() to simplify even more (like it's done for sessionId). Sorry if my first comment was not clear. Or there is a limitation that I don't see?
There was a problem hiding this comment.
Oh, I didn't realise Client from the Rust SDK had this value. Indeed, that's a lot easier.
There was a problem hiding this comment.
It seems like recording worked fine! Let's see how verification goes.
|
There was a problem hiding this comment.
It looks like this preview does not need to have the @PreviewWithLargeHeight, it's probably because of a copy paste from RoomDetailsView (which needs it). We may want to change that in another PR... or now. We may also want to keep it, for future need. We will have more action on this screen like "Verify user", "Jump to Read Receipt", etc.
jmartinesp
added
the
Run-Maestro
github-actions
bot
removed
the
Run-Maestro
Type of change
Content
@PreviewWithLargeHeightbehaviour so we can actually see the changes in the screenshot tests.Motivation and context
Fixes #2316 .
Screenshots / GIFs
Funnily enough, we should have screenshot changes due to this change, but as the screenshot only contains the top part of the screen, the changes are not visible ¯\(ツ)/¯.
Tests
Tested devices
Checklist