-
-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
[DOC] Added on modifier example to the input component #19205
Conversation
7f6f795
to
1f79d3e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great! Thank you!
@locks I tagged you since I think you know how/when to merge things in this repo. If not, can you tag another reviewer? |
@chancancode this PR aligns the API docs with what we landed on for the Guides ember-learn/guides-source#1546. If you could review & merge if it seems good, that would be great! Thank you! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do you think?
@@ -38,7 +38,13 @@ import InternalComponent from './internal'; | |||
|
|||
### Actions | |||
|
|||
The `Input` component takes a number of arguments with callbacks that are invoked in response to | |||
There are multiple ways to pass actions to the input component. Starting with Ember Octane, we recommend using the `{{on}}` modifier to call an action on specific events such as the input event. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are multiple ways to pass actions to the input component. Starting with Ember Octane, we recommend using the `{{on}}` modifier to call an action on specific events such as the input event. | |
The recommended way of handling DOM events is to use the `{{on}}` modifier. For example, to specify a callback for the `input` event: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we cross link to the documentation for the on modifier if that's available?
@@ -38,7 +38,13 @@ import InternalComponent from './internal'; | |||
|
|||
### Actions |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
### Actions | |
### Events |
<Input @value={{this.name}} {{on "input" this.validateName}} /> | ||
``` | ||
|
||
The `Input` component also takes a number of arguments with callbacks that are invoked in response to | ||
user events. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
user events. | |
other events. |
<Input @value={{this.name}} {{on "input" this.validateName}} /> | ||
``` | ||
|
||
The `Input` component also takes a number of arguments with callbacks that are invoked in response to | ||
user events. | ||
|
||
* `enter` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
* `enter` | |
* `enter` (synthetic event) |
IMO we should do this for this for all the synthetic events and at the end of the list mention what they are (something along the lines of "Some of these do not correspond to a DOM event. Instead, they are synthetic/logical events that are fired by the Input
component when a high-level user action is take (such as pressing the Enter key).")
Hey @stevetyler 👋 have you seen @chancancode 's suggestions? let us know if you need any help moving this forward 👍 |
Hey @stevetyler :) Do you have some time or do you need some help with the remaining work? |
#19314 was made (and merged) to address the same issue. Is this PR still needed? |
kk, good point @bertdeblock thanks for the reminder! Closing in favor of #19314 (thanks for everyones hard work here!!). |
#19201