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.

Sign up for GitHub

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

Jump to bottom

feat: improve UX for attaching to existing session #1607

Merged
merged 18 commits into from
Aug 18, 2024
Merged

feat: improve UX for attaching to existing session #1607

merged 18 commits into from
Aug 18, 2024

Conversation

eglitise
Copy link
Collaborator

This PR is a slight enhancement to the Attach to Session tab functionality:

  • Do not retrieve sessions when opening app or changing server parameters
  • Automatically retrieve sessions upon opening the Attach to Session tab
  • Move attachSessId calculation from reducers file to actions file
  • Do not auto-populate selected session if multiple sessions are found (unless a session was already selected)
  • List discovered sessions in reverse order (newest to oldest)
  • Add more details in session text (e.g. bundleId/appPackage if no app found; udid if no deviceName found)
    • I would also add timestamps, but the Appium server does not store this information
  • Add tooltip for refresh button
  • Update docs

Resolves #1585.

@github-actions github-actions bot added documentation Improvements or additions to documentation enhancement New feature or request labels Aug 15, 2024
KazuCocoa
KazuCocoa reviewed Aug 18, 2024
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.

Left a minor comment, otherwise lgtm. nice

app/common/renderer/actions/Session.js Outdated Show resolved Hide resolved
@eglitise
Copy link
Collaborator Author

While writing unit tests, I had to import the ServerTypes values, but since it was not properly defined as a const, I had to make changes in several other files.

mykola-mokhnach
mykola-mokhnach reviewed Aug 18, 2024
View reviewed changes
test/unit/utils-attaching-to-session.spec.js
@@ -0,0 +1,62 @@
import {describe, expect, it} from 'vitest';
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@eglitise eglitise merged commit d9ccb20 into appium:main Aug 18, 2024
7 checks passed
@eglitise eglitise deleted the improve-attaching-to-session branch August 18, 2024 17:00
KazuCocoa
KazuCocoa reviewed Aug 18, 2024
View reviewed changes
app/common/renderer/constants/session-builder.js
@@ -1,3 +1,28 @@
export const SESSION_BUILDER_TABS = {
Copy link
Member

Choose a reason for hiding this comment

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

👍

laib3 pushed a commit to laib3/appium-inspector that referenced this pull request Nov 16, 2024
@eglitise
feat: improve UX for attaching to existing session (appium#1607)
39d0e30 
* fix: do not set attachSessId to first session if multiple are found

* feat: auto-refresh session list when switching to attach to session tab
also remove session retrieval anywhere outside this tab

* chore: remove unused import

* fix: add tooltip to button for reloading session discovery

* chore: run prettier

* feat: show more details for discovered sessions
also reverse the list to show most recent sessions first

* docs: update attach to session docs

* chore: revert session discovery reload button changes

* fix: do not add platformVersion if there is no platformName

* chore: rename vars for readability

* fix: reset attachSessId for Sauce or error

* chore: address review comments

* chore: use an OOP approach

* chore: add consts for session builder tabs

* chore: add consts for server types

* chore: move session info assembly to utils

* test: add unit tests for session info assembly

* chore: refactor super() call
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@KazuCocoa KazuCocoa KazuCocoa left review comments

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

Assignees
No one assigned
Labels
documentation Improvements or additions to documentation enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

feature request: Improved functionality on the "Attach to Session" tab
3 participants
@eglitise @KazuCocoa @mykola-mokhnach