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

[FEATURE modernized-built-in-components] First-pass implementation #19218

Merged
merged 2 commits into from Jan 28, 2021

Conversation

chancancode
Copy link
Member

@chancancode chancancode commented Oct 20, 2020

First-pass implementation for converting the <Input> component into an internal (not classic) component compatibly. This passes all the existing tests.

Some follow-up work before this is ready for prime time:

  • Implement any remaining classic component features not covered by the existing tests.

  • Detect any unimplementable classic component features and deopt into the previous implementation.

  • Write the second deprecation RFC and get the proposal merged.

@chancancode chancancode force-pushed the refactor-input-component branch 5 times, most recently from c13355a to 656f72f Compare October 27, 2020 06:33
@chancancode chancancode changed the title [FEATURE modernized-built-in-components] WIP implementation [FEATURE modernized-built-in-components] First-pass implementation Oct 27, 2020
@chancancode chancancode marked this pull request as ready for review October 27, 2020 06:34
@chancancode
Copy link
Member Author

This passes all the existing tests and is ready for review – at least it will be a useful starting point for some conversation. IMO the remaining work can be done in follow-up PRs. I'm also not sure whether we want to do a very rigious audit on what Ember.Component features or just do the main ones we know about and let people report bugs of any untested features. There are things like @tagName which technically works but I don't know that anyone would do that intentionally on the <Input> component (that is a more valid/expected thing to do to the <LinkTo> component, on the other hand).

@chancancode chancancode force-pushed the refactor-input-component branch 2 times, most recently from 63f6024 to c403f53 Compare October 27, 2020 07:20
`Passing the \`@${argument}\` argument to <Input> is deprecated. ` +
`Instead, please pass the attribute directly, i.e. \`<Input ${attribute}={{...}} />\` ` +
`instead of \`<Input @${argument}={{...}} />\` or \`{{input ${argument}=...}}\`.`,
true /* TODO !(argument in this.args) */,
Copy link
Member Author

Choose a reason for hiding this comment

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

Actually enabling all these deprecations require fixing lots of tests, so leave them disabled for now.

packages/@ember/-internals/glimmer/lib/components/input.ts Outdated Show resolved Hide resolved
packages/@ember/-internals/glimmer/lib/templates/input.hbs Outdated Show resolved Hide resolved
`the <Input> component and prevented it from functioning properly. ` +
`Instead, please use the {{on}} modifier, i.e. \`<Input {{on "${eventName}" ...}} />\` ` +
`instead of \`<Input @${methodName}={{...}} />\` or \`{{input ${methodName}=...}}\`.`,
true /* TODO !descriptor */,
Copy link
Member

Choose a reason for hiding this comment

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

Can you open an issue with some guidance and then link back to it from here? Basically, we need some path to enabling this deprecation...

Copy link
Member Author

Choose a reason for hiding this comment

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

Comment on lines +57 to +81
{{!-- deprecated native event callbacks --}}
{{on "touchstart" this._touchStart}}
{{on "touchmove" this._touchMove}}
{{on "touchend" this._touchEnd}}
{{on "touchcancel" this._touchCancel}}
{{on "keydown" this._keyDown}}
{{on "keypress" this._keyPress}}
{{on "mousedown" this._mouseDown}}
{{on "mouseup" this._mouseUp}}
{{on "contextmenu" this._contextMenu}}
{{on "click" this._click}}
{{on "dblclick" this._doubleClick}}
{{on "focusin" this._focusIn}}
{{on "focusout" this._focusOut}}
{{on "submit" this._submit}}
{{on "dragstart" this._dragStart}}
{{on "drag" this._drag}}
{{on "dragenter" this._dragEnter}}
{{on "dragleave" this._dragLeave}}
{{on "dragover" this._dragOver}}
{{on "drop" this._drop}}
{{on "dragend" this._dragEnd}}
{{on "mouseenter" this._mouseEnter}}
{{on "mouseleave" this._mouseLeave}}
{{on "mousemove" this._mouseMove}}
Copy link
Member

Choose a reason for hiding this comment

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

We should either read the event dispatcher for its values here, or just defer to it

Copy link
Member Author

Choose a reason for hiding this comment

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

I actually wrote the code to address this and submitted #19269. However, as explained in that PR, I don't want to introduce a new global name for a one-off thing we are making here. Given that we expect the contextual modifier RFC to land pretty soon and this is still behind a flag, I would prefer to punt on addressing this for now.

Copy link
Member Author

Choose a reason for hiding this comment

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

First-pass implementation for converting the `<Input>` component
into an internal (not classic) component compatibly. This passes
all the existing tests.

Some follow-up work before this is ready for prime time:

- Implement any remaining classic component features not covered
  by the existing tests.

- Detect any unimplementable classic component features and deopt
  into the previous implementation.

- Write the second deprecation RFC and get the proposal merged.
@pzuraq pzuraq merged commit 0f49c4e into master Jan 28, 2021
@pzuraq pzuraq deleted the refactor-input-component branch January 28, 2021 22:49
eramod added a commit to eramod/ember.js that referenced this pull request Feb 4, 2021
…ation

Tracking issue: emberjs#19270
More specifically, it enables the <Input> attribute deprecations introduced
in emberjs#19218, fixes broken tests, and
updates all test {{input}} invocations to use <Input> instead.

Deprecate input arguments
eramod added a commit to eramod/ember.js that referenced this pull request Feb 4, 2021
…ation

Tracking issue: emberjs#19270
More specifically, it enables the <Input> attribute deprecations introduced
in emberjs#19218, fixes broken tests, and
updates all test {{input}} invocations to use <Input> instead.

Deprecate input arguments
eramod added a commit to eramod/ember.js that referenced this pull request Feb 4, 2021
…ation

Tracking issue: https://github.com/emberjs/ember.js/issues/192t 70
More specifically, it enables the <Input> attribute deprecations introduced
in emberjs#19218, fixes broken tests, and
updates all test {{input}} invocations to use <Input> instead.

WIP - add maybeExpectDeprecation function
eramod added a commit to eramod/ember.js that referenced this pull request Feb 4, 2021
…ation

Tracking issue: https://github.com/emberjs/ember.js/issues/192t 70
More specifically, it enables the <Input> attribute deprecations introduced
in emberjs#19218, fixes broken tests, and
updates all test {{input}} invocations to use <Input> instead.

WIP - add maybeExpectDeprecation function
eramod added a commit to eramod/ember.js that referenced this pull request Feb 4, 2021
…ation

Tracking issue: https://github.com/emberjs/ember.js/issues/192t 70
More specifically, it enables the <Input> attribute deprecations introduced
in emberjs#19218, fixes broken tests, and
updates all test {{input}} invocations to use <Input> instead.

WIP - add maybeExpectDeprecation function
eramod added a commit to eramod/ember.js that referenced this pull request Feb 5, 2021
…ation

Tracking issue: https://github.com/emberjs/ember.js/issues/192t 70
More specifically, it enables the <Input> attribute deprecations introduced
in emberjs#19218, fixes broken tests, and
updates all test {{input}} invocations to use <Input> instead.

WIP - add maybeExpectDeprecation function
eramod added a commit to eramod/ember.js that referenced this pull request Feb 6, 2021
…ation

Tracking issue: https://github.com/emberjs/ember.js/issues/192t 70
More specifically, it enables the <Input> attribute deprecations introduced
in emberjs#19218, fixes broken tests, and
updates all test {{input}} invocations to use <Input> instead.

WIP - add maybeExpectDeprecation function
chancancode pushed a commit to eramod/ember.js that referenced this pull request Feb 6, 2021
…ation

Tracking issue: https://github.com/emberjs/ember.js/issues/192t 70
More specifically, it enables the <Input> attribute deprecations introduced
in emberjs#19218, fixes broken tests, and
updates all test {{input}} invocations to use <Input> instead.

WIP - add maybeExpectDeprecation function
chancancode pushed a commit to eramod/ember.js that referenced this pull request Feb 6, 2021
…ation

Tracking issue: emberjs#19270

More specifically, it enables the <Input> attribute deprecations introduced
in emberjs#19218, fixes broken tests, and
updates all test {{input}} invocations to use <Input> instead.
eramod added a commit to eramod/ember.js that referenced this pull request Feb 10, 2021
Tracking issue: emberjs#19270
Enables the input event handlers deprecation introduced in emberjs#19218
and fixes resultant failing tests
eramod added a commit to eramod/ember.js that referenced this pull request Feb 10, 2021
Tracking issue: emberjs#19270
Enables the input event handlers deprecation introduced in emberjs#19218
and fixes resultant failing tests
eramod added a commit to eramod/ember.js that referenced this pull request Feb 10, 2021
Tracking issue: emberjs#19270
Enables the input event handlers deprecation introduced in emberjs#19218
and fixes resultant failing tests
eramod added a commit to eramod/ember.js that referenced this pull request Feb 10, 2021
Tracking issue: emberjs#19270
Enables the input event handlers deprecation introduced in emberjs#19218
and fixes resultant failing tests
eramod added a commit to eramod/ember.js that referenced this pull request Feb 10, 2021
…nt handlers

Tracking issue: emberjs#19270
Enables the input and textarea  event handlers deprecation introduced in emberjs#19218
and fixes resultant failing tests
eramod added a commit to eramod/ember.js that referenced this pull request Feb 10, 2021
…nt handlers

Tracking issue: emberjs#19270
Enables the input and textarea  event handlers deprecation introduced in emberjs#19218
and fixes resultant failing tests
eramod added a commit to eramod/ember.js that referenced this pull request Feb 10, 2021
…nt handlers

Tracking issue: emberjs#19270
Enables the input and textarea  event handlers deprecation introduced in emberjs#19218
and fixes resultant failing tests
chancancode pushed a commit that referenced this pull request Feb 11, 2021
…nt handlers

Tracking issue: #19270
Enables the input and textarea  event handlers deprecation introduced in #19218
and fixes resultant failing tests
chancancode pushed a commit that referenced this pull request Feb 13, 2021
…nt handlers

Tracking issue: #19270
Enables the input and textarea  event handlers deprecation introduced in #19218
and fixes resultant failing tests
chancancode pushed a commit that referenced this pull request Feb 25, 2021
…nt handlers

Tracking issue: #19270
Enables the input and textarea  event handlers deprecation introduced in #19218
and fixes resultant failing tests
sly7-7 pushed a commit to sly7-7/ember.js that referenced this pull request Mar 10, 2021
…ation

Tracking issue: emberjs#19270

More specifically, it enables the <Input> attribute deprecations introduced
in emberjs#19218, fixes broken tests, and
updates all test {{input}} invocations to use <Input> instead.
sly7-7 pushed a commit to sly7-7/ember.js that referenced this pull request Mar 10, 2021
…nt handlers

Tracking issue: emberjs#19270
Enables the input and textarea  event handlers deprecation introduced in emberjs#19218
and fixes resultant failing tests
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.

None yet

3 participants