Skip to content

Conversation

@Develog
Copy link
Contributor

@Develog Develog commented Oct 15, 2025

Q A
Bug fix? no
New feature? yes
Deprecations? no
Documentation? yes
Issues no
License MIT

Next to this PR #2712 I wanted to add test assertion on dispatchBrowserEvent and not only on emited event.
I reuse same logic as the ComponentWithEmit.php.

$this->assertComponentDispatchEvent($testComponent->render(), 'browserEvent')
    ->withPayload(['amount' => 2, 'unit' => 'kg'])
    ->withPayloadSubset(['amount' => 2]) ;

$this->assertComponentNotDispatchEvent($testComponent->render(), 'otherBrowserEvent');

@Develog Develog force-pushed the feat/add-dispatch-browser-event-test branch from cba34d0 to ab52681 Compare October 15, 2025 06:02
@Develog Develog marked this pull request as ready for review October 15, 2025 06:06
@carsonbot carsonbot added Documentation Improvements or additions to documentation Feature New Feature LiveComponent Status: Needs Review Needs to be reviewed labels Oct 15, 2025
@Develog
Copy link
Contributor Author

Develog commented Oct 15, 2025

I'm asking myself if it's better to create a dedicated component that only have one action that dispatch browser event rather than reuse ComponentWithEmit.php

@smnandre
Copy link
Member

Nice!

Copy link
Member

@Kocal Kocal left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution!

Except for the naming, it's good. Could you add an entry for 2.31 in the LiveComponent CHANGELOG.md?

Thanks

@carsonbot carsonbot added Status: Needs Work Additional work is needed and removed Status: Needs Review Needs to be reviewed labels Oct 21, 2025
@Develog Develog force-pushed the feat/add-dispatch-browser-event-test branch from ab52681 to b4618e8 Compare October 21, 2025 20:13
@carsonbot carsonbot added Status: Needs Review Needs to be reviewed and removed Status: Needs Work Additional work is needed labels Oct 21, 2025
@Develog Develog force-pushed the feat/add-dispatch-browser-event-test branch from b4618e8 to a345d81 Compare October 21, 2025 20:15
@Develog Develog force-pushed the feat/add-dispatch-browser-event-test branch from a345d81 to 5943c89 Compare October 21, 2025 20:20
@Develog
Copy link
Contributor Author

Develog commented Oct 21, 2025

@Kocal I fixed my PR following your review but I have one test that fail and I don't understand why. Do you have any idea ?

@Kocal
Copy link
Member

Kocal commented Oct 21, 2025

Thanks!

The issue comes from Foundry, it has been reported in zenstruck/foundry#1011 (comment) and zenstruck/foundry#1012

@carsonbot carsonbot added Status: Reviewed Has been reviewed by a maintainer and removed Status: Needs Review Needs to be reviewed labels Oct 22, 2025
@Kocal
Copy link
Member

Kocal commented Oct 22, 2025

Thank you @Develog.

@Kocal Kocal merged commit 375415b into symfony:2.x Oct 22, 2025
18 of 28 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Documentation Improvements or additions to documentation Feature New Feature LiveComponent Status: Reviewed Has been reviewed by a maintainer

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants