feat: add php port to ports - #3108
Conversation
dazgreer
left a comment
There was a problem hiding this comment.
Hi, thanks for the PR and for adding a PHP port of MJML.
I think this is potentially a useful addition to the ports documentation, but I'd like to suggest a couple of changes before we add it:
- Testing against the current MJML release
The repository's snapshot tests currently use MJML 5.2.1 as their JavaScript reference. With the latest MJML release being 5.4.0, could you please update them to use this version and confirm the results?
I ran this independently and 17 of the 20 fixtures pass after applying the test's existing normalisation. The three failures appear to come down to two relatively small differences:
- multi-column: the PHP implementation produces 200px / 400px where MJML 5.4.0 produces 199.98px / 400.02px for the 33.33% / 66.67% columns.
- social and full-email: the JS implementation adds border:0 to the social icon image styles, which is absent from the PHP output.
Could you please investigate these differences and either bring the PHP output into line with MJML 5.4.0 or consider documenting why the differences are intentional?
The existing test suite itself passes successfully on my setup (262 tests, 689 assertions), so this is specifically about ensuring the port is being compared against the current MJML implementation rather than the older 5.2.1 reference.
- Documentation
For your PR specifically, I'd also suggest separating the documentation into distinct mjml-php and Laravel sections, rather than having the Laravel integration included within the mjml-php section. I think this would make it clearer that these are two separate packages/integrations.
I ported MJML to PHP