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

Setting placeholder for a component is broken #12514

Closed
quaertym opened this issue Oct 22, 2015 · 9 comments
Closed

Setting placeholder for a component is broken #12514

quaertym opened this issue Oct 22, 2015 · 9 comments
Labels

Comments

@quaertym
Copy link
Contributor

What I try to do is to localize placeholder text for a component in its willInsertElement hook by passing localization key as a parameter to the component. Same code was working with 1.12.x but it is broken in 1.13.x and 2.x.x versions. This twiddle demonstrates the problem. Is this a bug? Or am I doing something wrong?

Ember.STRINGS = {
    '_hello_': 'Bonjour'
}

// components/localized-input.js
export default Ember.Component.extend({
  tagName:'input',
  attributeBindings: ['placeholder'],
  willInsertElement: function() {
    this._super();
    var placeholder = this.get('placeholder');
    if (placeholder) {
        this.set('placeholder', placeholder.loc());
    }
  }
});

// template
{{localized-input placeholder="_hello_"}}
@pixelhandler
Copy link
Contributor

@quaertym I think you should not be using the willInsertElement hook and instead just use a computed property like so : http://ember-twiddle.com/76eb01ec99bc9ed5f621

import Ember from 'ember';

Ember.STRINGS = {
    '_hello_': 'Bonjour'
}

export default Ember.Component.extend({
  tagName:'input',
  attributeBindings: ['prompt:placeholder'],
  prompt: Ember.computed('placeholder', function() {
    let prompt = this.get('placeholder');
    return Ember.String.loc(prompt);
  })
});

@quaertym
Copy link
Contributor Author

@pixelhandler This fixes it but does not explain why it suddenly broke.

@stefanpenner
Copy link
Member

As @pixelhandler points out, using a CP is correct, as there are several issues in not doing so:

  • setting during willInsert causes re-render to occur during render.
  • read + set during such a live-cycle hook, breaks bindings

That being said, this does appear to indicate an issue. I believe the issue is a collision between placeholder attribute (being passed in) and the placeholder property. If one changes the attribute from placeholder to anything else ... maybe placeholder-loc="_hello" and updated the gets to use that, everything once again works.

@rwjblue any thing come to mind?

@rwjblue
Copy link
Member

rwjblue commented Oct 26, 2015

Yes, I do believe this is a bug (but the OP should still change to a CP as it is better for this sort of thing).

The bug is basically that we do not seem to use changes to things in attributeBindings during willInsertElement (my WAG is that we grab attributeBinding values before calling willInsertElement so setting during the same run loop doesn't properly trigger those to be dirtied and updated).

@stefanpenner
Copy link
Member

(my WAG is that we grab attributeBinding values before calling willInsertElement so setting during the same run loop doesn't properly trigger those to be dirtied and updated).

Ya this is likely the issue. Although I would prefer users not use this functionality, this does seem like a bug.

cc @tomdale / @wycats

@runspired
Copy link
Contributor

Likely related to #11480

@karthiicksiva
Copy link
Contributor

Issue in updating attributeBindings value after v1.12.x. here is the twiddle.

Earlier in 1.12.x, we have updated the attributeBindings regardless of consumer mentioning the attribute and this is not the case from v1.13+. It is broken here.

cc @rwjblue

@Serabe
Copy link
Member

Serabe commented Apr 18, 2016

Closing in favor of #11480. This workaround seems to work in your case as well.

Thank you for reporting!

@Serabe Serabe closed this as completed Apr 18, 2016
@karthiicksiva
Copy link
Contributor

@Serabe yeah that workaround would work for normal component, but not for the extended one.

For e.g: I have a component, where I have extended Textarea component. In this case, unfortunately the workaround won't work(attributeBindings for rows).

At the moment, I directly update the DOM attribute, even though it's a bad way. Any suggestions..??

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

No branches or pull requests

7 participants