Skip to content
This repository has been archived by the owner on Apr 25, 2023. It is now read-only.

feat: add locator timings #1448

Merged
merged 4 commits into from
Jul 5, 2020
Merged

feat: add locator timings #1448

merged 4 commits into from
Jul 5, 2020

Conversation

wswebcreation
Copy link
Contributor

The goal of this PR is to give people insights on the time it will take to locate elements with a certain locator strategy. It will add:

  • a button in the inspector window to get times of finding elements with a certain locator strategy
  • a spinner for waiting
  • find element times in the Find by table

Android example
appium-times-android

iOS example:
appium-times-ios

@wswebcreation
feat: add locator timings
88c9c0d
@wswebcreation
Merge branch 'master' into feat/add-locator-timings
6b2b145 
# Please enter a commit message to explain why this merge is necessary,
# especially if it merges an updated upstream into a topic branch.
#
# Lines starting with '#' will be ignored, and an empty message aborts
# the commit.
@mykola-mokhnach
Copy link
Contributor

I think from UX perspective it is not very obvious that Get Times button should fill up these cells. Would it be possible to put such button or a link directly into the cell where the resulting number should appear?

@wswebcreation
Copy link
Contributor Author

I think from UX perspective it is not very obvious that Get Times button should fill up these cells. Would it be possible to put such button or a link directly into the cell where the resulting number should appear?

@mykola-mokhnach

I've added a button to make it more clear like this, the tool tip is still present.

image

image

@mykola-mokhnach
Copy link
Contributor

I think Timing would be a more precise term here. Get Time has a bit different meaning to me (although I'm not a native speaker, so it might be a wrong assumption)

@wswebcreation
Copy link
Contributor Author

I think Timing would be a more precise term here. Get Time has a bit different meaning to me (although I'm not a native speaker, so it might be a wrong assumption)

Change it to this, including the tool tip

image

KazuCocoa
KazuCocoa approved these changes Jul 5, 2020
View reviewed changes
Copy link
Member

@KazuCocoa KazuCocoa left a comment

Choose a reason for hiding this comment

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

👍

@KazuCocoa KazuCocoa merged commit 803a85c into appium:master Jul 5, 2020
@wswebcreation wswebcreation deleted the feat/add-locator-timings branch July 5, 2020 13:09
@wswebcreation
Copy link
Contributor Author

Tnx for merging @KazuCocoa

Can we get a new release in the coming days with those 2 new features?

@KazuCocoa
Copy link
Member

Perhaps yes.
Once bump current version like f0ffcdd and run a CI job, the new one will override https://github.com/appium/appium-desktop/releases/tag/v1.18.0-beta.0 (Or we should increment the version)
Let me try

@KazuCocoa
Copy link
Member

@wswebcreation
Current master is available from https://github.com/appium/appium-desktop/releases/tag/v1.18.0-beta.1 as pre release

@wswebcreation
Copy link
Contributor Author

@KazuCocoa

Thanks!! It's working, just one question, can you also add this PR #1448 in the description? The Get Timings is also merged and in this release

@KazuCocoa
Copy link
Member

👍 added

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@KazuCocoa KazuCocoa KazuCocoa approved these changes

@mykola-mokhnach mykola-mokhnach mykola-mokhnach approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
3 participants
@wswebcreation @mykola-mokhnach @KazuCocoa