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

feat: Current window position #130

Merged

Conversation

Squidonomics
Copy link
Contributor

@Squidonomics Squidonomics commented Dec 3, 2023

Implements the following functions:
/session/:sessionId/window/:windowHandle/maximize
/session/:sessionId/window/:windowHandle/position

Sources/WebDriver/Session.swift Outdated Show resolved Hide resolved
Tests/UnitTests/APIToRequestMappingTests.swift Outdated Show resolved Hide resolved
Copy link
Contributor

@tristanlabelle tristanlabelle left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution! We can design this API to have a closer shape to the Element API. I left some comments.

Sources/WebDriver/Requests.swift Outdated Show resolved Hide resolved
Sources/WebDriver/Requests.swift Outdated Show resolved Hide resolved
Sources/WebDriver/Session.swift Outdated Show resolved Hide resolved
Sources/WebDriver/Session.swift Outdated Show resolved Hide resolved
Sources/WebDriver/Session.swift Outdated Show resolved Hide resolved
Sources/WebDriver/Session.swift Outdated Show resolved Hide resolved
Tests/UnitTests/APIToRequestMappingTests.swift Outdated Show resolved Hide resolved
Tests/UnitTests/APIToRequestMappingTests.swift Outdated Show resolved Hide resolved
Tests/UnitTests/APIToRequestMappingTests.swift Outdated Show resolved Hide resolved
Copy link
Contributor

@tristanlabelle tristanlabelle left a comment

Choose a reason for hiding this comment

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

Thanks for the update, the new Window struct is looking good, we just need an accessor from Session so that we don't need to duplicate members there.

Sources/WebDriver/Window.swift Outdated Show resolved Hide resolved
Sources/WebDriver/Window.swift Outdated Show resolved Hide resolved
Sources/WebDriver/Requests.swift Outdated Show resolved Hide resolved
Sources/WebDriver/Session.swift Outdated Show resolved Hide resolved
Sources/WebDriver/Window.swift Outdated Show resolved Hide resolved
Sources/WebDriver/Window.swift Show resolved Hide resolved
@Squidonomics
Copy link
Contributor Author

My most recent commit should address all the comments. Sorry for the delay I've been running through the interview circuit so not a lot of free time.

Copy link
Contributor

@tristanlabelle tristanlabelle left a comment

Choose a reason for hiding this comment

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

Thanks for iterating! The docs now need to be updated but the code looks good!

Docs/SupportedAPIs.md Outdated Show resolved Hide resolved
@tristanlabelle
Copy link
Contributor

Attempted a merge in the GitHub actions editor, hopefully I didn't mess it up.

@tristanlabelle
Copy link
Contributor

Oops I messed up the merge by adding public var windowHandle: String to SessionOrientation.Get. I'm not sure I can change it. @Squidonomics, could you remove it?

@Squidonomics
Copy link
Contributor Author

Just pushed up a fix for that @tristanlabelle this is ready to go.

@tristanlabelle tristanlabelle merged commit 8626746 into thebrowsercompany:main Mar 5, 2024
1 check passed
@Squidonomics Squidonomics deleted the squid/maximum-window branch March 6, 2024 20:55
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