Skip to content
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

fix: set value attribute on textarea instead of in content to ensure value and defaultValue stay in sync with changes #447

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/spotty-lamps-shake.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@crowdstrike/ember-toucan-core': patch
---

Update Textarea to ensure underlying <textarea> value stays in sync with both typing and @value changes
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,8 @@ export default class ToucanFormTextareaControlComponent extends Component<Toucan
readonly={{@isReadOnly}}
...attributes
{{on "input" this.handleInput}}
>{{@value}}</textarea>
Copy link
Contributor

Choose a reason for hiding this comment

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

@simonihmig I can't recall exactly why we did this initially. Do you?

value={{@value}}
></textarea>
</div>
</template>
}
58 changes: 57 additions & 1 deletion test-app/tests/integration/components/textarea-test.gts
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
/* eslint-disable no-undef -- Until https://github.com/ember-cli/eslint-plugin-ember/issues/1747 is resolved... */
import { fillIn, render } from '@ember/test-helpers';
import { fillIn, render, settled } from '@ember/test-helpers';
import { module, test } from 'qunit';

import TextareaControl from '@crowdstrike/ember-toucan-core/components/form/controls/textarea';
import { setupRenderingTest } from 'test-app/tests/helpers';
import { tracked } from '@glimmer/tracking';
import { action } from '@ember/object';

module('Integration | Component | Textarea', function (hooks) {
setupRenderingTest(hooks);
Expand Down Expand Up @@ -78,6 +80,60 @@ module('Integration | Component | Textarea', function (hooks) {
assert.dom('[data-textarea]').hasValue('tony');
});

test('it keeps the <textarea> value in sync with external changes to `@value`', async function (assert) {
class TestContext {
@tracked testValue;
}
const testContext = new TestContext();
testContext.testValue = 'initial';

await render(<template>
{{! we do not require a label, but instead suggest using Field / TextareaField }}
{{! template-lint-disable require-input-label }}
<TextareaControl @value={{testContext.testValue}} data-textarea />
</template>);

assert.dom('[data-textarea]').hasValue('initial');

testContext.testValue = 'updated';

await settled();

assert.dom('[data-textarea]').hasValue('updated');
});

test('it keeps the <textarea> value in sync with external changes to `@value` after the value is changed', async function (assert) {
class TestContext {
@tracked testValue;

@action
updateTestValue(value, e) {
this.testValue = value;
}
}
const testContext = new TestContext();
testContext.testValue = 'initial';

await render(<template>
{{! we do not require a label, but instead suggest using Field / TextareaField }}
{{! template-lint-disable require-input-label }}
<TextareaControl @value={{testContext.testValue}} @onChange={{testContext.updateTestValue}} data-textarea />
</template>);

assert.dom('[data-textarea]').hasValue('initial');

await fillIn('[data-textarea]', 'test');

assert.dom('[data-textarea]').hasValue('test');

testContext.testValue = 'updated';

await settled();

assert.dom('[data-textarea]').hasValue('updated');
});


test('it calls `@onChange` when input is received', async function (assert) {
assert.expect(6);

Expand Down
Loading