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

[Textfield] add name attribute and reflect input value to element attribute #46

Closed
wants to merge 3 commits into from

Conversation

eskan
Copy link
Contributor

@eskan eskan commented May 25, 2018

Hi Liters,
a PR to improve text-field component, and get it work with "iron-form":

  • add the name attribute to the embedded input.
  • reflect input value to element value attribute.
  • fire value-changed event.
  • fire change event when shadowRoot

Regards,

@eskan eskan changed the title add name attribute and reflect input value to element attribute [Textfield] add name attribute and reflect input value to element attribute May 25, 2018
@pitus
Copy link

pitus commented May 27, 2018

I see a commit with an input on-change call _updateValue() but how can I bind to this change in my own component using the mwc-textfield? The on-change doesn't fire for mwc-textfield, what's the right way to do this?

Also, it would be useful to be able to handle the on-input changes as well, not just on-change, so that the user can get real-time feedback in some cases as they're typing.

@eskan
Copy link
Contributor Author

eskan commented May 27, 2018

@pitus .. you're right .. i'll update this PR with on-input instead of on-change.
i mostly use this component in form but i agree this element should fire event.

@markcellus
Copy link

Just out of curiosity, cant the value attribute be binded to?

@eskan
Copy link
Contributor Author

eskan commented May 27, 2018

@mkay581 i guess you think about a 2 ways binding like Polymer 1,2 did but with lit and lit-element that doesn't work.
when a property change, _render is called to apply change to the dom : "no intelligence here" or "hidden magic".
if you want to display the value of the input somewhere else you need to do something like this :

_render({_myInput}) {
return html`<label>${_myInput}</labe>
<mwc-textfield on-input=${(e) => this._myInput = e.target.value}></mwc-textfield>`
}

@markcellus
Copy link

markcellus commented May 27, 2018

hmm, on the Polymer 3 site, it shows an example of data binding using lit-html.

since the code in this component assigns the new value to value property, why wouldn't I be able to do this with the current implementation?

return html`<label>{{_myInput}}</label>
  <mwc-textfield value="{{_myInput}}"></mwc-textfield>`
}

EDITED: small syntax change

@eskan
Copy link
Contributor Author

eskan commented May 27, 2018

@mkay581 .. ok, i understand the confusion, Polymer and lit-html both use html`` helpers :
https://www.polymer-project.org/3.0/docs/devguide/dom-template but they are totally different.
My bad, i thought only with LitElement :/ so if you use Polymer-Element you'll be able to bind value with this PR if you use LitElement you should write it.

@pitus
Copy link

pitus commented May 28, 2018

@eskan For some reason on-change doesn't fire for mwc-textfield while the on-input does. I've added both to the mwc-textfield the same way as in the recent commit - they both fire within the mwc-textfield calling my _setName(e) but only on-input is fired within my own control (which adds the on-input to the mwc-textfield). What's 'blocking' the on-change event, why doesn't it propagate? What am I missing?

I have the same problem with the mwc-checkbox where the on-change doesn't get fired (my component is not notified of change in the checkbox) but ... if I add an on-click handler (both, to my code and within the mwc-checkbox to call its _inputChangeHandler()) then it all works fine ...

So what's wrong with my using <mwc-XXX on-change="${this._handler}">... Why don't they get fired?

@eskan
Copy link
Contributor Author

eskan commented Jun 7, 2018

@pitus i've updated this PR to allow change and value-changed events, may you test this ?

@eskan eskan mentioned this pull request Jun 12, 2018
Copy link

@omar-belkhodja-bacdev omar-belkhodja-bacdev left a comment

Choose a reason for hiding this comment

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

Is it normal that you didn't use "composed: true" in the CustomEvent ?

@eskan
Copy link
Contributor Author

eskan commented Jul 27, 2018

@omar-belkhodja-bacdev : input change event isn't a composed event so i reproduce this behavior

@eskan
Copy link
Contributor Author

eskan commented Jan 30, 2019

outdated

@eskan eskan closed this Jan 30, 2019
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

Successfully merging this pull request may close these issues.

4 participants