Skip to content

pressKey: Fixing @param string|array<string|string> $chars #138

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

Merged
merged 3 commits into from
Dec 15, 2024

Conversation

ThomasLandauer
Copy link
Member

@ThomasLandauer ThomasLandauer commented Nov 17, 2024

The docblock was changed in #88, and was probably intended to be string|string[]

@ThomasLandauer
Copy link
Member Author

@TavoNiievez Can you merge this please?

@TavoNiievez TavoNiievez merged commit 57611e1 into Codeception:master Dec 15, 2024
1 of 4 checks passed
@ThomasLandauer ThomasLandauer deleted the patch-1 branch December 15, 2024 21:03
@ThomasLandauer
Copy link
Member Author

Thanks! Do you know why the tests are failing for all PRs?

@indy2kro
Copy link

Umm, guys, array<string, string> is not quite the same as string[] which includes basically array<int, string> and array<int, string>.
In particular the way the code works right now it accepts an array with strings inside indeed, but the keys don't make any sense to be string, e.g.

        $I->pressKey('#input', ['ctrl', 'a']);

But the definition implies something like this:

        $I->pressKey('#input', ['huh' => 'ctrl', 'what' => 'a']);

So I would suggest to just put string[] if not sure, but probably array<int, string> would make more sense.

@ThomasLandauer
Copy link
Member Author

Indeed, sorry, looks like I thought the syntax was ['ctrl' => 'a']. So I'm changing it to list<string> now in #141 which is basically the more modern way of saying string[] ;-)

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.

3 participants