-
Notifications
You must be signed in to change notification settings - Fork 159
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
Issue 699 compose doc loc metadata #700
base: master
Are you sure you want to change the base?
Conversation
|
||
private fun getLocalizedStrings(): List<LocalizedString> { | ||
uiDevice.setCompressedLayoutHierarchy(false) | ||
val objectsWithText = uiDevice.findObjects(By.enabled(true)) |
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.
100% needs better UiObjects tree parsing. Probably would be better if we used recursion
@@ -1,6 +1,7 @@ | |||
package com.kaspersky.kaspresso.docloc | |||
|
|||
import com.kaspersky.kaspresso.device.screenshots.screenshotmaker.ScreenshotMaker | |||
import com.kaspersky.kaspresso.docloc.metadata.saver.MetadataSaver |
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.
only import without usage?
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 is used in the consctructor. Before my changes MetadataSaver was in the same pachage as DocLocScreenshotCapturer. I made renamed it to the DefautMetadataSaver and extracted the interface MetadataSaver. Now it has to be imported
for (obj in objectsWithText) { | ||
try { | ||
val resName = obj.resourceName ?: continue | ||
if (!obj.text.isNullOrBlank()) { |
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.
if you have else block you do not need !
val localizedStrings = mutableListOf<LocalizedString>() | ||
for (obj in objectsWithText) { | ||
try { | ||
val resName = obj.resourceName ?: continue |
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.
maybe it will be better to throw exception? Or we really need to skip item without res name?
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.
The idea is to pick the view that might be the text container or a text itself. Either way we need an id which might be nullable, but I don't think we have to throw an exception
obj.children | ||
.filter { !it.text.isNullOrBlank() } | ||
.forEach { | ||
val coords = obj.visibleBounds |
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 code can be moved to a separate method
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 found a possible bug. When we iterate through the children we better use the child coordinates
Closes #399