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.

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

style: enhance find element modal #842

Merged
merged 33 commits into from
May 26, 2023

Conversation

eglitise
Copy link
Collaborator

This PR provides various visual improvements to the find element modal:

  • Replace locator strategy dropdown with radio buttons (one less click and no scroll)
  • Hide locator strategies that are unavailable for current automationType
  • Render results count using antd Badge
  • Render results list with responsive height (1-6 rows)
  • Replace text for found element action buttons with icons + tooltip
  • Add a clear button to selector and send keys input fields if text is entered

Here is a demonstration of the new behavior:

refreshed-find-element-walkthrough.mp4

@eglitise eglitise requested review from jlipps and KazuCocoa May 23, 2023 12:32
@mykola-mokhnach
Copy link
Contributor

mykola-mokhnach commented May 23, 2023

Would it make sense to sort location strategies by performance (e.g. most performant go first)?

@eglitise
Copy link
Collaborator Author

It probably could be done, but I don't see much benefit to it. As a design, the proposed 2-row radio button style (in my opinion) doesn't really imply any sorting, unlike the current dropdown design. And different automation engines may then require different list orders, which could be annoying when working with more than one engine.

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.

lgtm 👍

Copy link
Member

@jlipps jlipps left a comment

Choose a reason for hiding this comment

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

great idea! made a few suggestions

app/renderer/components/Inspector/ElementLocator.js Outdated Show resolved Hide resolved
app/renderer/components/Inspector/LocatedElements.js Outdated Show resolved Hide resolved
@eglitise eglitise merged commit 498b118 into appium:main May 26, 2023
@eglitise eglitise deleted the refresh-find-element-modal branch May 26, 2023 06:33
shiva-guntoju pushed a commit to shiva-guntoju/appium-inspector that referenced this pull request Feb 2, 2024
* fix: do not list unavailable locator strategies

* style: increase height of selector text area

* feat: add clear button to selector input field

* style: replace strategy selector with segmented

* fix: add missing localised string

* style: render found elem count as badge

* style: replace segmented with radio buttons

* fix: center strategy button group for non-mobile

* fix: show XCUI strategies for Mac drivers

* fix: detect buttons based on automation name

* style: improve design of located elem actions

* style: improve locator strategy button look

* style: adjust button group border/highlight

* fix: replace almost duplicate translation key

* style: match selected item color to source tree

* fix: add missing loading state during search

* style: separate tap button from send keys group

* fix: clear element highlight when pressing back
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants