-
Notifications
You must be signed in to change notification settings - Fork 739
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.
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
[Device management] Session details screen (PSG-686) #7084
Conversation
super.bind(holder) | ||
holder.sessionDetailsContentTitle.text = title | ||
holder.sessionDetailsContentDescription.text = description | ||
holder.sessionDetailsContentDescription.copyOnLongClick() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should be able to long press on the whole view.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in 9a8acb9
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code seems really clean, just some small remarks, not blocking
@@ -3265,6 +3265,14 @@ | |||
<string name="device_manager_session_title">Session</string> | |||
<!-- Examples: Last activity Yesterday at 6PM, Last activity Aug 31 at 5:47PM --> | |||
<string name="device_manager_session_last_activity">Last activity %1$s</string> | |||
<string name="device_manager_session_details_title">Session details</string> | |||
<string name="device_manager_session_details_description">Application, device, and activity information.</string> | |||
<string name="device_manager_session_details_section_session_title">Session</string> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suggest having a reusable string name for this common field, wdyt?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree, I will reuse the existing device_manager_session_title
. Sometimes, I duplicate the strings since this is not the same screen and so not the same context. It could be changed more easily after. But in this case I guess it will not change.
<string name="device_manager_session_details_session_name">Session name</string> | ||
<string name="device_manager_session_details_session_id">Session ID</string> | ||
<string name="device_manager_session_details_session_last_activity">Last activity</string> | ||
<string name="device_manager_session_details_section_device_title">Device</string> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will create a more general strings for this title: device_manager_device_title
.
<string name="device_manager_session_details_session_id">Session ID</string> | ||
<string name="device_manager_session_details_session_last_activity">Last activity</string> | ||
<string name="device_manager_session_details_section_device_title">Device</string> | ||
<string name="device_manager_session_details_device_ip_address">IP address</string> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It could also be the case for this one, "Session ID" and "Session name" which are already used in other places but it is maybe less relevant
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I renamed the existing string with content "Session ID" but I didn't do the same for session name as we don't have the same translation right now. We will see later if we want to use the same. Updated in b36a699
val deviceFullInfo = getDeviceFullInfoUseCase.execute(A_DEVICE_ID).firstOrNull() | ||
|
||
deviceFullInfo shouldBeEqualTo Optional(null) | ||
// Then | ||
deviceFullInfo shouldBeEqualTo null |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you can call deviceFullInfo.shouldBeNull()
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated in d138718
b36a699
to
fb3fdf0
Compare
SonarCloud Quality Gate failed. |
Type of change
Content
Adding a new screen to show session details. It can be reached from the session overview screen. For now, we only have sections for non-extended data - session name, device id, IP, latest activity.
Motivation and context
Closes #7077
Screenshots / GIFs
Tests
Tested devices
Checklist