stimulusreflex / stimulus_reflex

Build reactive applications with the Rails tooling you already know and love.
https://docs.stimulusreflex.com
MIT License
2.29k stars 174 forks source link

Alternative fix for 652 #692

Closed tomclose closed 6 months ago

tomclose commented 7 months ago

Type of PR (feature, enhancement, bug fix, etc.)

Description

Fixes the handling of

morph '#foo', "<div id='foo'><input><span>Keep me!</span></div>

by making the following tradeoff:

morph '#foo', "<tr id='foo'><td>1</td></tr></tr>" # this won't work (results in "1" being sent as html)
morph '#foo', "<td>1</td>" # but this will work (i.e results in "<td>1</td>" being sent as html)

Note: this is largely a revert of https://github.com/stimulusreflex/stimulus_reflex/commit/69cb070ec961dd52d34ec9a66a88f895686100c1

Fixes #652

Why should this be added

652 is a hard to debug and is easy to run into e.g. when building an autocomplete search box. The real fix for the bug involves an upstream change to Nokogiri, but it's unclear when that will happen. I don't know if this is a good tradeoff or not - just thought I'd propose it as an alternative to #674.

Checklist

netlify[bot] commented 7 months ago

Deploy Preview for stimulusreflex ready!

Name Link
Latest commit 8db047de66993af764c366d726662eae57e844b8
Latest deploy log https://app.netlify.com/sites/stimulusreflex/deploys/660b43d698240a000806ba81
Deploy Preview https://deploy-preview-692--stimulusreflex.netlify.app
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

marcoroth commented 6 months ago

@tomclose just wanted to tag you and mention that https://github.com/stimulusreflex/stimulus_reflex/pull/696 should be solving this issue! If you get a chance, I'd love to hear how it works for you! Thank you!