Repository navigation
[internal] feat(locator): press(Keys...) and focus() for #134 - #2
crystalmountain666 merged 3 commits into
Conversation
crystalmountain666
left a comment
There was a problem hiding this comment.
Static review of the three commits at f941837.
The implementation looks consistent with the existing Locator API. press(Keys...) forwards the supplied keys without clearing the value, and focus() uses JavaScript without clicking the element. The focusable-element requirement is documented.
Both methods reuse whenVisible() and the existing whenReady() polling logic. Resolution and action execution happen inside the retry loop, so stale references during the action are retried rather than treated as success.
The tests extend the existing action matrix instead of duplicating it. The stale-during-action cases verify that the retry acts on a freshly resolved element, while the persistent-stale cases require a timeout. The key arguments and JavaScript focus call are checked, and the pointer-action coverage is retained.
The Javadoc, changelog and semantic-locators guide are consistent with the implementation.
The supplied Maven log reports 774 tests, 4 failures, 0 errors and 0 skipped. All listed failures are in CucumberQuarantineTest; none are listed for LocatorActionsTest. I have not independently run the build.
Before approval, please record the base-branch comparison for those four failures and the documentation build result. Before the upstream PR, please also complete the real-page validation required by seleniumboot#134.
I have no implementation changes to request from this static review; the remaining items concern verification.
What & why
Internal review PR within our fork. This adds the keyboard and focus half of seleniumboot#134:
Locator.press(Keys...)andLocator.focus(), together with tests and documentation.It builds on the pointer actions already merged in #1. The documentation covers all four actions for our joint upstream PR.
Commits
feat(locator): add press(Keys...) and focus() (#134)press(Keys...)sends the supplied keys without clearing the element's existing value.focus()focuses a focusable element using JavaScript without clicking it.Both methods wait for visibility and reuse
whenVisible(), following the existing terminal-action contract: re-resolve on every poll and retry stale or not-yet-present elements untiltimeouts.explicit, including staleness during the action. Both methods include Javadoc and usage examples.test(locator): cover press and focus (#134)Extends the existing
LocatorActionsTestdata provider from two actions to four. This adds 16 keyboard/focus test invocations, bringing the class to 33 in total: eight scenarios × four actions, plus the existing double-click enabled-wait test.The scenarios cover ready, late and hidden elements; staleness during readiness checks and during the action; elements that never appear; persistent staleness; and elements that never become visible.
The added checks verify the requested keys and JavaScript focus call on the resolved element. Existing pointer-action coverage is retained.
docs(locator): document locator actions (#134)Adds an Unreleased changelog entry covering all four actions. Updates the semantic-locators guide with the keyboard, focus and pointer methods, a
Keys.ENTERexample, and their automatic waiting and re-resolution behaviour.Verification
mvn clean verify -Dgpg.skip=trueon macOS.CucumberQuarantineTest:checkQuarantine_underJUnitPlatform_throwsTestAbortedException_whenInYamlcheckQuarantine_underJUnitPlatform_throwsTestAbortedException_whenTaggedcheckQuarantine_underTestNg_throwsSkipException_whenInYamlcheckQuarantine_underTestNg_throwsSkipException_whenTaggedLocatorActionsTestfailures are listed in the full-run summary.Outstanding verification:
npm run buildindocs-siteand attach the documentation build result.selenium-boot-test.Please check
press(Keys...)preserve the existing value and send the requested keys correctly?focus()use the resolved element without clicking it, with the focusable-element requirement clearly documented?Merge with Rebase and merge to preserve the three separate commits and their authorship.
@crystalmountain666 could you review this when you have time? Thanks!