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

Add small extensions #30

Merged
merged 4 commits into from
Jan 19, 2024
Merged

Add small extensions #30

merged 4 commits into from
Jan 19, 2024

Conversation

M1troll
Copy link
Contributor

@M1troll M1troll commented Jan 18, 2024 •
edited
Loading

  • Add | (or) operator for XPathLocators
  • Add Page.click_to_page method
  • Update WebView.init_element to be consistent with ComponentWithBaseLocator.init_element

Update:

After discussion, we decided to revert init_element changes back to minimize code needed for simple cases. Instead, we added a recomendation to documentation that when using this method it is better to write locator= by default, but if it gets in the way in simple cases, it can be ignored.

@M1troll M1troll self-assigned this Jan 18, 2024
@M1troll M1troll force-pushed the feature/add-small-extensions branch from 3e34652 to b527e9b Compare January 18, 2024 07:00
@M1troll M1troll marked this pull request as ready for review January 18, 2024 07:03
TheSuperiorStanislav
TheSuperiorStanislav approved these changes Jan 18, 2024
View reviewed changes
docs/CHANGELOG.rst Outdated Show resolved Hide resolved
ron8mcr
ron8mcr reviewed Jan 18, 2024
View reviewed changes
Copy link
Member

@ron8mcr ron8mcr left a comment

Choose a reason for hiding this comment

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

I'm not sure about making locator argument be keyword-only.
The less code required for simple cases - the better, so I suggest to revert this change back

pomcorn/locators/base_locators.py Outdated Show resolved Hide resolved
pomcorn/page.py Outdated Show resolved Hide resolved
@M1troll M1troll force-pushed the feature/add-small-extensions branch from b527e9b to d77ee57 Compare January 18, 2024 10:43
@M1troll M1troll force-pushed the feature/add-small-extensions branch from d77ee57 to 50760e3 Compare January 18, 2024 10:47
@M1troll M1troll requested a review from ron8mcr January 18, 2024 10:49
@M1troll M1troll merged commit 63daae8 into main Jan 19, 2024
2 checks passed
@M1troll M1troll deleted the feature/add-small-extensions branch January 19, 2024 09:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ron8mcr ron8mcr ron8mcr approved these changes

@TheSuperiorStanislav TheSuperiorStanislav TheSuperiorStanislav approved these changes

@NikAzanov NikAzanov Awaiting requested review from NikAzanov NikAzanov is a code owner

Assignees

@M1troll M1troll

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@M1troll @ron8mcr @TheSuperiorStanislav