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

Allow dusk attribute selectors to be chained #1034

Merged
merged 2 commits into from
Apr 13, 2023

Conversation

JayBizzle
Copy link
Contributor

@JayBizzle JayBizzle commented Apr 13, 2023

I have come across a few situations when writing Dusk tests where I use the dusk attribute selector on a wrapper div, then want to select elements within that div...

e.g.

<div dusk="all-items">
    <div>Item 1</div>
    <div>Item 2</div>
    <div>Item 3</div>
    <div>Item 4</div>
</div>

And I was hoping I could do something like this...

$items = $browser->elements('@all-items > div');

foreach($items as $item){
    // assert something
}

...but this doesn't work because the selector gets compiled to [dusk="all-items > div"]

This PR makes this selector get compiled to [dusk="all-items"] > div

Bonus

Also allows multiple dusk attribute selectors to work as expected

This selector...

@item-list > div:nth-child(2) @submit-button

becomes...

[dusk="item-list"] > div:nth-child(2) [dusk="submit-button"]

@JayBizzle JayBizzle marked this pull request as ready for review April 13, 2023 09:10
@taylorotwell taylorotwell merged commit 836338e into laravel:7.x Apr 13, 2023
@patrickomeara
Copy link

This has broken dusk selectors with spaces.

@JayBizzle
Copy link
Contributor Author

This has broken dusk selectors with spaces.

Thanks for reporting, will submit a fix when I get the the office in a couple of hours 👍🏻

@patrickomeara
Copy link

@JayBizzle No troubles. In my opinion the functionality that you have added here is worth the breaking change. I'd be interested to see how wide spread the space issue is. Swapping my spaces out won't be too much work, it's mainly for selecting submenus in react components. A breaking change nonetheless. Thanks for the quick response.

@JayBizzle
Copy link
Contributor Author

JayBizzle commented Apr 19, 2023

Just working on this now. Allowing spaces isn't an issue.

Characters we can't allow (that I can see using regex) are CSS chaining chars like ., >, & etc otherwise things like this won't work...

@dusk selector .class
@dusk selector > div

PR will be ready in about 10 mins

@patrickomeara
Copy link

patrickomeara commented Apr 19, 2023

I think that would go against the descendant CSS selector spec. ie if you are trying to target descendants of a selector

<div dusk="all-items">
    <div>Item 1</div>
    <div>Item 2</div>
    <div>Item 3</div>
    <div>Item 4</div>
</div>

You could rightfully use @all-items div, using spaces would break this @all items div.

@JayBizzle
Copy link
Contributor Author

Yes, that is the problem I have hit with the solution. There doesn't seem to be a way around that unfortunately.

See #1035 to continue discussion 👍

@driesvints
Copy link
Member

@patrickomeara seems Taylor has decided to continue with this PR, sorry.

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