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

Assertion Failed: You modified "value" twice in a single render #67

Open
makepanic opened this issue Jul 24, 2019 · 3 comments
Open

Assertion Failed: You modified "value" twice in a single render #67

makepanic opened this issue Jul 24, 2019 · 3 comments

Comments

@makepanic
Copy link
Contributor

It seems like ember-pickr has issues when used as data-down, action-up component.

onSave(color) {
  this.set('value', color.toHEXA().toString());
}
{{color-picker value=value onSave=(action onSave)}}

It might be possible to workaround this by telling pickr to update silently.

Failing test:

  test('it handles the color value changes without dying', async function(assert) {
    this.set('value', '#bada55');
    this.set('onSave', (color) => this.set('value', color.toHEXA().toString()));
    await render(hbs`{{color-picker value=value onSave=(action onSave)}}`);
    await sleep(1000);

    this.set('value', '#11ccee');

    assert.equal(
      getComputedStyle(this.element.querySelector('.pcr-button')).color,
      'rgb(17, 204,238)'
    );
  });
@makepanic
Copy link
Contributor Author

makepanic commented Jul 29, 2019

Had some time to investigate this.
It seems like the issue in the example above, is that the value is passed in as lowercase but pickr normalizes hex to uppercase.

Meaning that it has the case where #11ccee is not equal to #11CCEE so it renders again.

@astronomersiva
Copy link
Owner

astronomersiva commented Jul 30, 2019

Sorry, missed this issue somehow.

It seems like the issue in the example above, is that the value is passed in as lowercase but pickr normalizes hex to uppercase.

That's strange. If I am understanding this correctly, we could convert all values to uppercase before handing them over to pickr and prevent this. What are your thoughts on this?

@makepanic
Copy link
Contributor Author

That's strange. If I am understanding this correctly, we could convert all values to uppercase before handing them over to pickr and prevent this

yes that seems to fix the failing test above

Sadly even with heavily controlled color strings I've seen the same error when switching colors. I haven't had time to reproduce that issue in a failing test.

Maybe we should rethink the value mutation and make it use data down, action up more closely.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants