Updated Built-In Components page to clarify how to use <Input> component

[ijlee2]

Description

This PR addresses and can close issue #1543 . In particular, in the Ember Learning Team's Input component plans document, this PR completes 3 action items:

• Take out the parts that talk about what doesn't work (for example, "Checkboxes are a special input type..." section
• Present modifiers as the main way to do events
• Show example of {{on "input}} or {{on "change"}} handling w/o addons in the first pass. Maybe later show keydown (For simplicity in code, I showed how to migrate @enter and @escape-press using ember-keyboard)

I read everything in Built-In Components to figure out how to update each subsection and its examples, so that the updated page will be coherent as a whole.

I also created a new Ember v3.21.1 app. I copy-pasted the existing code samples to figure out what worked and what didn't. The updated code samples should work when a developer copy-pastes them to a new app (assuming v3.21).

How to Review

I recommend reviewing commits one by one. Each commit looks at one subsection in Built-In Components.

Notes

In meeting, @chancancode, @jenweber, @locks, and I weren't sure whether, with <Input> component, a developer needed to use @id (argument) or id (attribute) to correctly bind to the input element's id attribute.

Based on my v3.21.1 app, I think we have to use id. (This is good for us, as it requires fewer changes.)

Consider the following code. It changes the input ID every 1 second.

{{!-- app/templates/application.hbs --}}
<label for={{this.myUniqueId}}>
  Name:
</label>

<Input
  id={{this.myUniqueId}}
  @value={{this.name}}
/>

<div {{did-insert this.changeUniqueId}}></div>

// app/controllers/application.js

import Controller from '@ember/controller';
import { action } from '@ember/object';
import { later } from '@ember/runloop';
import { tracked } from '@glimmer/tracking';

export default class ApplicationController extends Controller {
  @tracked myUniqueId = 'happy-little-trees';
  @tracked name = 'Bob Ross';

  @action changeUniqueId() {
    later(() => {
      this.myUniqueId = Math.random().toString();
      this.changeUniqueId();
    }, 1000);
  }
}

When we use <Input id=... />, in Chrome Inspector, we will see that the ID changes for the for and id attributes every second. Clicking on the label will focus on the input as well.

However, when we use <Input @id=... />, we will see that the ID changes only for the for attribute in the label. Clicking on the label will not focus on the input anymore.

[chancancode]

I think we probably need to do more testing. It is in fact illegal to change the ID of an ember component after its instantiated (and probably a bit sketchy though valid on HTML elements in general), which is why it doesn’t update using your method (I think you are supposed to get an assertion).

I think this isn’t that common in practice and instead we should focus on whether things like event delegation still works if you use the ID attribute. ie does the focusIn(), etc hooks defined on the component still fire correctly.

[ijlee2]

@chancancode Sounds good, thanks for the quick feedback. I'll look at event delegation more closely and let you know how it goes.

[ijlee2]

@chancancode Here is my updated example. I hope I understood what you were looking for:

Template

{{!-- app/templates/application.hbs --}}

<label for="version-1">Name:</label>
<Input
  id="version-1"
  @value={{this.name1}}

  @mouse-down={{this.mouseDown}}
  @mouse-up={{this.mouseUp}}
  @context-menu={{this.contextMenu}}
  @click={{this.click}}
  @double-click={{this.doubleClick}}

  @change={{this.change}}
  @focus-in={{this.focusIn}}
  @focus-out={{this.focusOut}}
  @input={{this.input}}
/>


<label for="version-2">Name:</label>
<Input
  id="version-2"
  @value={{this.name2}}

  @mouseDown={{this.mouseDown}}
  @mouseUp={{this.mouseUp}}
  @contextMenu={{this.contextMenu}}
  @click={{this.click}}
  @doubleClick={{this.doubleClick}}

  @change={{this.change}}
  @focusIn={{this.focusIn}}
  @focusOut={{this.focusOut}}
  @input={{this.input}}
/>


<label for="version-3">Name:</label>
<Input
  id="version-3"
  @value={{this.name3}}

  {{on "mousedown" this.mouseDown}}
  {{on "mouseup" this.mouseUp}}
  {{on "contextmenu" this.contextMenu}}
  {{on "click" this.click}}
  {{on "dblclick" this.doubleClick}}

  {{on "change" this.change}}
  {{on "focusin" this.focusIn}}
  {{on "focusout" this.focusOut}}
  {{on "input" this.input}}
/>

Controller

// app/controllers/application.js

import Controller from '@ember/controller';
import { action } from '@ember/object';
import { tracked } from '@glimmer/tracking';

export default class ApplicationController extends Controller {
  @tracked name1 = 'Bob Ross';
  @tracked name2 = 'Bob Ross';
  @tracked name3 = 'Bob Ross';


  // https://api.emberjs.com/ember/release/classes/Component#event-handler-methods
  @action mouseDown(event) {
    console.log('mousedown');
    console.log(event);
  }

  @action mouseUp(event) {
    console.log('mouseup');
    console.log(event);
  }

  @action contextMenu(event) {
    console.log('contextmenu');
    console.log(event);
  }

  @action click(event) {
    console.log('click');
    console.log(event);
  }

  @action doubleClick(event) {
    console.log('dblclick');
    console.log(event);
  }


  @action change(event) {
    console.log('change');
    console.log(event.target.value);
  }

  @action focusIn(event) {
    console.log('focusin');
    console.log(event);
  }

  @action focusOut(event) {
    console.log('focusout');
    console.log(event);
  }

  @action input(event) {
    console.log('input');
    console.log(event.target.value);
  }
}

I created 3 sets of label-input:

1. Uses event arguments with dasherized names (we want these to work)
2. Uses event arguments with camel-cased names (we want these to not work)
3. Uses {{on}} modifiers (we want these to work)

In v3.21.1, not all events for 1st input fire in the expected order, while all events for the 2nd input do fire in the expected order (at least, as far as I can tell). The events for 3rd input do work as expected. The results for all 3 inputs seem to be the same whether I use id or @id.

Perhaps we need to test in v3.16 LTS instead (bugfix emberjs/ember.js#19001)? From the bugfix emberjs/ember.js#18997, I wasn't sure in which recent Ember version the fix landed. (landed in v3.20.0)

[chancancode]

Yeah that seems like a good test to me. The fix should be available in v3.21.1 so I would expect it to work. Are you able to:

1. Set a breakpoint around here https://github.com/emberjs/ember.js/blob/bd15e2d1d76278712dc3051b766149e31eefe26a/packages/%40ember/-internals/views/lib/mixins/text_support.js#L302 and see if it's called for the first input, and if so, then
2. Follow it into here https://github.com/emberjs/ember.js/blob/bd15e2d1d76278712dc3051b766149e31eefe26a/packages/%40ember/-internals/views/lib/mixins/text_support.js#L310 and follow it and see why it didn't end up calling the callback?

Otherwise you can push your app somewhere and I can take a look too

[ijlee2]

I uploaded my demo app and recorded what I tried in README: https://github.com/ijlee2/investigate-input-events-in-3.21

I don't get to walk through the ember-source often enough. Can you explain how to set a breakpoint for sendAction (a line of code that's not immediately available to me in my app, if this make sense)?

[chancancode]

@ijlee2 you can do it from the chrome devtools.

1. Go to the "Sources" pane in the Chrome dev tools.
2. Press "Command-P" to open the "Open File" dialog (you can find it in the UI buried in some menu too).
3. If you have source maps enabled, you can probably type in text_support and find the source file I linked. Otherwise, you'll probably have to open vendor.js and search the source text for the AMD module ("@ember/-internals/views/lib/mixins/text_support") using Command-F (full text search).
4. Either way, once you found the line, you can click on the line number to set a breakpoint.

[chancancode]

(Instead of keyDown, you probably want to pick one that you actually bound in the template in your test. The focus events are okay, but they tend to be quite finicky because clicking on the devtools will cause the input to unfocus.)

[chancancode]

I think I see the problem in your test: you tried to do @mouse-down etc but those aren't actually arguments supported by the component. @foo-bar isn't a general purpose way of registering callbacks. For example, @key-down works, but only because the component is explicitly documented to support it and has code that dispatches the callback. Otherwise, it won't do anything.

Then, things like @click, @change, etc basically "works" by "overriding" the methods on the component with the same name which as we discussed is problematic.

[ijlee2]

@chancancode Thanks very much for the instructions. I was able to set a breakpoint on sendAction and trigger the breakpoint for keydown event when I typed a character into the input field for Versions 1 and 3, but not Version 2.

Changes I made to the sample app

I think you're right that we have to use @id to bind to the <Input> component's elementId. If we use id instead, the component's elementId is the default, unique string ember###.

It's also worth noting that version 3 ({{on}} modifier approach) does not create key-down, key-press, and key-up properties in the attrs object. I imagine this supports the idea that {{on}} modifier is a better approach.

Screenshots

Version 1, @id

Version 3, @id

Version 1, id

Version 3, id

[chancancode]

I was able to set a breakpoint on sendAction and trigger the breakpoint for keydown event when I typed a character into the input field for Versions 1 and 3, but not Version 2.

👍 I think that all went as expected. Version 2 doesn’t trip the breakpoint because the passed argument overwrote and replaced the handler on the component, which is The Problem™️.

I think you're right that we have to use @id to bind to the component's elementId. If we use id instead, the component's elementId is the default, unique string ember###.

Yeah so that’s the problem/quirk of classic components in general. However, the fact that you can get version 1 working as expected either way means that Ember internally no longer uses elementId for the purpose of event dispatching, which was what I was originally worried that this would break.

If this is your own component or something from an addon that you are invoking, you may care about elementId being correct for other reasons, but it seems like the built-in Input component doesn’t actually use it for anything or otherwise care about its value? In that case, even though it is more Correct™️ to use @id we may be able to get away without it in this case and still not cause any real issues.

It's also worth noting that version 3 ({{on}} modifier approach) does not create key-down, key-press, and key-up properties in the attrs object. I imagine this supports the idea that {{on}} modifier is a better approach.

Yep that is also as expected. The {{on}} modifier doesn’t interact with the component at all and instead operate on the underlying DOM element (the <input> in this case) via ...attributes (which is implicit on the wrapper element for classic components). So the component doesn’t “see” these event handlers at all.

[ijlee2]

Awesome. Other than a review of the writing, this PR should be good to go then?

[jenweber]

Hello! Thanks so much for writing this! It is a big improvement for our users.

I added a few comments and wording changes. The ones that I would say are required are, we should not say that any particular addon is recommended, if linking to a list of them will suffice. The others are around cutting down references to Ember Classic and "legacy." We have some of that language sprinkled through the Guides to help give people a glide path, but Octane has been out for a while now and I think we should walk towards removing the references. These may not be described in a styleguide anywhere - I will try to fix that.

[jenweber]

Looks great, thank you!