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

[Feature] Add waitForClickable helper #635

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

[Feature] Add waitForClickable helper #635

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
94b72ad
Add WaitForClickable helper
JoppeDC Jul 25, 2024
7be9a40
Merge branch 'symfony:main' into patch-1
JoppeDC Dec 7, 2024
96b95fc
Revert "Add WaitForClickable helper"
JoppeDC Dec 7, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions src/Client.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -507,6 +507,23 @@ public function waitForDisabled(string $locator, int $timeoutInSecond = 30, int
return $this->crawler = $this->createCrawler();
}

/**
* @param string $locator The path to the element that will be disabled. Can be a CSS selector or Xpath expression.
*
* @throws NoSuchElementException
* @throws TimeoutException
*/
public function waitForClickable(string $locator, int $timeoutInSecond = 30, int $intervalInMillisecond = 250): PantherCrawler
Copy link
Member

Choose a reason for hiding this comment

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

$timeoutInSecond -> $timeoutInSeconds ?

same for $intervalInMillisecond -> $intervalInMilliseconds ?

Copy link
Author

@JoppeDC JoppeDC Jul 25, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I thought this was a little weird too, but this is the way its done for all the other cases as well.

Cant change it for the other cases either, as thats a BC. We keep it like this so they're the same, or adjust this one?

Copy link
Member

Choose a reason for hiding this comment

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

I see. The same arguments in other methods are indeed named in the same way you did.

Changing the argument names can be made in other PR if wanted. By the way, Symfony's BC promise doesn't cover argument names, so we can change them if we decide it's better to do so. See https://symfony.com/doc/current/contributing/code/bc.html#note-10

Thanks

Copy link
Author

Choose a reason for hiding this comment

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

Awesome, TIL!

I think its best to change all the names in a seperate PR in that case :)

{
$by = self::createWebDriverByFromLocator($locator);

$this->wait($timeoutInSecond, $intervalInMillisecond)->until(
WebDriverExpectedCondition::elementToBeClickable($by)
);

return $this->crawler = $this->createCrawler();
}

public function getWebDriver(): WebDriver
{
$this->start();
Expand Down
Loading