search

stimulusreflex / stimulus_reflex

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

Text area values are lost if re-sized #195

Closed joshleblanc closed 4 years ago

joshleblanc commented 4 years ago

Describe the bug

When a form is re-rendered to add a new field, the existing values will persist. However, if one of the inputs is a text area and the text area has been resized, the value will be lost.

To Reproduce

MVCE repo: https://github.com/joshleblanc/stimulus-reflex-test

Expected behavior

The value shouldn't be lost.

Screenshots or reproduction

ElvIw0hbh0

Versions

StimulusReflex

External tools

Browser

leastbad commented 4 years ago

Wow, this library never fails to provide ways to get more intimate with the weird shit that browsers do. :)

Thanks Josh, we're on it. Good catch! Are you on Discord?

joshleblanc commented 4 years ago

Just joined

hopsoft commented 4 years ago

Great find and excellent bug report. Thanks.

railscard commented 4 years ago

This is indeed an interesting issue. I've checked the code and found that current behaviour of the morph action may cause other similar problems since every element modified by external plugins (like select2) will be morphed on every reflex call.

For now, I can see two ways to solve the original issue:

  1. Ignore style attribute for textareas in cableready, but it doesn't look right.

  2. Add data-reflex-permanent attribute to an element. Such way may not work for everyone, mainly because it makes impossible to remove association later, so we might slightly change an API to make it possible to assign there option like allow-discard.

leastbad commented 4 years ago

Hi @railscard! Are you on Discord? hint

Ignoring style attributes isn't a desirable solution, as you said.

I'm not sure what you're picturing with data-reflex-permanent in this scenario. Marking sections as permanent removes a lot of flexibility and utility - it's designed for things like Google AdWords and yes, jQuery plugins like select2.

That said, I would strongly recommend switching to more modern alternatives such as choices.js. I have a Stimulus wrapper that makes choices work great in a Turbolinks context and even adds Ajax lookups, all without modifying the original library.

In the meantime, this is almost certainly a morphdom bug. I wish they'd treat it as such!

leastbad commented 4 years ago

@joshleblanc on ln 25 of app/views/foos/_form.html.erb you appear to be emitting an ActiveRecord model into a text field, as well as an extra %>.

Fixing this by removing the extra closing tag and changing it to foo.id doesn't solve the larger issue, sadly.

leastbad commented 4 years ago

Okay, I finally figured this out. There's no bug; it's working exactly as expected.

I spent a few good hours manually stepping through morphdom processing your view, recording to video and hitting F9 about 3000 times. I don't regret this at all, because it was my decision, I like a good mystery and I learned a lot.

There's actually two things happening here in a cascade that happen so quickly we perceive it as one thing, but that's why we're confused. The clue came from the realization that you're not persisting the text values in between reflex operations. It's actually just morphdom respecting the text node that is a child of the textarea. This wouldn't have happened if it was just an input element.

When you drag the resize handle, the browser is dynamically updating the CSSStyleSheetDeclaration object of the textarea, which presents as the style property. You are not communicating these styles to the server via data-attributes, so when the server re-renders your view, there is no style attribute on the markup.

When morphdom is recursing through your DOM, it is running element.isSameNode(newElement) on every element, and if it finds that your textarea has a style attribute on it, it will see it as an entirely new node even though it has the same name and id attributes.

Now your old textarea is swapped with a brand new textarea node, because this is far faster than updating the existing node as it will only require one draw call. This means that the text node child of the old textarea is gets its nodeValue set to ''.

Mystery solved. You'll need to put a simple Stimulus controller on your textarea that updates a data-style attribute with a serialized snapshot of your style property, and then make sure to pass that as a style attribute on your textarea markup.