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

Add @dusk blade directive to render dusk hooks in markup #347

Closed
wants to merge 2 commits into
base: 2.0
from

Conversation

Projects
None yet
3 participants
@calebporzio
Contributor

calebporzio commented Aug 29, 2017

This PR adds a blade directive to render dusk="" attribute "hooks" in markup:

<div @dusk('foo')></div>
<div dusk="foo"></div> // compiles to

Note: because DuskServiceProvider isn't loaded in production, these directives will not be rendered in production, therefore leaving behind @dusk('') in rendered html. I will submit a PR to laravel/framework that compiles an empty string when @dusk is encountered in production.

@deleugpn

This comment has been minimized.

Show comment
Hide comment
@deleugpn

deleugpn Aug 29, 2017

Contributor

Have you tested if this overrides an existing directive on the framework?

Contributor

deleugpn commented Aug 29, 2017

Have you tested if this overrides an existing directive on the framework?

@calebporzio

This comment has been minimized.

Show comment
Hide comment
@calebporzio

calebporzio Aug 29, 2017

Contributor

@deleugpn - yes

Contributor

calebporzio commented Aug 29, 2017

@deleugpn - yes

@taylorotwell

This comment has been minimized.

Show comment
Hide comment
@taylorotwell

taylorotwell Aug 29, 2017

Member

If we're going to have a dusk directive in the framework anyways you might as well just put it there haha :)

Member

taylorotwell commented Aug 29, 2017

If we're going to have a dusk directive in the framework anyways you might as well just put it there haha :)

@calebporzio

This comment has been minimized.

Show comment
Hide comment
@calebporzio

calebporzio Aug 29, 2017

Contributor

Ugh @taylorotwell, sorry that this is still a thing.

TLDR:

// with this PR
@dusk('foo') // only renders when DuskServiceProvider loads (desired behavior every time)

// without this PR
@dusk('foo') // only renders on `local` and `testing` environments
@dusk('foo', 'some other environment') // we don't need the extra, optional param with this PR

I actually think it's better to have both. This way we don't have to guess which environments the users want the dusk="" attributes rendered in (local, testing). They only load when DuskServiceProvider loads. It also makes the code much simpler because there's no expression parsing for environments and conditionals for rendering.

Let me know though and I'll just re-PR the other laravel/framework only implementation.

Contributor

calebporzio commented Aug 29, 2017

Ugh @taylorotwell, sorry that this is still a thing.

TLDR:

// with this PR
@dusk('foo') // only renders when DuskServiceProvider loads (desired behavior every time)

// without this PR
@dusk('foo') // only renders on `local` and `testing` environments
@dusk('foo', 'some other environment') // we don't need the extra, optional param with this PR

I actually think it's better to have both. This way we don't have to guess which environments the users want the dusk="" attributes rendered in (local, testing). They only load when DuskServiceProvider loads. It also makes the code much simpler because there's no expression parsing for environments and conditionals for rendering.

Let me know though and I'll just re-PR the other laravel/framework only implementation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment