-
Notifications
You must be signed in to change notification settings - Fork 4.5k
[UI] Remove send Action deprecation #22938
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
base: main
Are you sure you want to change the base?
Conversation
791375a
to
4aed01e
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.
Pull Request Overview
This PR removes deprecated sendAction
usage from the Consul UI codebase to comply with Ember.js deprecation guidelines. The changes replace the deprecated sendAction
pattern with modern Ember action handling using @action
decorators and direct function calls.
Key changes include:
- Converting component controllers from classic Ember syntax to modern class-based syntax
- Replacing
sendAction
calls with direct action method calls - Updating template action handlers to use
queue
andfn
helpers for proper action composition
Reviewed Changes
Copilot reviewed 22 out of 22 changed files in this pull request and generated 2 comments.
Show a summary per file
File | Description |
---|---|
ui/packages/consul-ui/app/controllers/dc/acls/tokens/edit.js | Converted to modern class syntax and added use and delete action methods |
ui/packages/consul-ui/app/controllers/dc/acls/roles/edit.js | Added delete action method and updated constructor |
ui/packages/consul-ui/app/controllers/dc/acls/policies/edit.js | Added delete action method and updated constructor |
ui/packages/consul-ui/app/components/confirmation-dialog/index.js | Converted to modern class syntax and removed sendAction dependency |
ui/packages/consul-ui/app/components/popover-menu/index.js | Removed deprecated send action method |
Multiple template files | Updated action handlers to use queue and fn helpers instead of sendAction |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
ui/packages/consul-lock-sessions/app/components/consul/lock-session/form/index.hbs
Show resolved
Hide resolved
@color='critical' | ||
data-test-delete | ||
{{on 'click' (fn execute)}} | ||
{{on 'click' (queue execute (fn writer.delete @item))}} |
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.
I realize that there is no backing class for this component currently but in the near future it would be great to really on ember-composible-helpers
like queue
and all other in most places where it's possible. For now this is fine as intermediate step 👍🏻
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.
I didn't quite catch that. Can you give me an example?
@color='critical' | ||
data-test-delete | ||
{{on 'click' (fn confirm (fn writer.delete @item))}} | ||
{{on 'click' (fn confirm )}} |
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.
I might be missing some context here, why was this moved
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.
The ConfirmationDialog
uses confirm method to accept a method and params to call when the execute function is called. The method and args passed are saved in the component's scope. Then the execute method uses sendAction to call the passed method.
I am removing that dependency by directly calling the method along with execute function.
{{/if}} | ||
{{#if (gt items.length 0)}} | ||
<YieldSlot @name="set">{{yield}}</YieldSlot> | ||
<YieldSlot @name="set" @params={{block-params (action "remove")}}>{{yield}}</YieldSlot> |
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.
action
helper is deprecated as well but I am not sure if it would work in the new format in the current ember version, it should be just either @remove
or this.remove
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.
I tried using this.remove
along with updating the class file to native JS class. Neither of them didn't work.
I suspect it's the Slotted mixin which could be causing the issue.
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.
no worries we can address it later on 👍🏻
data-test-delete | ||
disabled={{api.disabled}} | ||
{{on 'click' (action confirm api.delete)}} | ||
{{on 'click' (action confirm)}} |
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.
same as above, but we might want to address in it's own PR
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.
it looks good 👍🏻 I left a few FYI comments
The base branch was changed.
fbea107
to
b7b3728
Compare
4aed01e
b7b3728
to
4aed01e
Compare
41e6f63
to
c72538f
Compare
bda8947
to
c72538f
Compare
c72538f
to
3ab8852
Compare
* replaces partials with components * added readme files * add tests * remove unused variable * added changelog * updated changelog * fixes linting * fixes lnting * delete unused partial files and code cleanup * Update 22888.txt * Update README.mdx
Description
As per the deprecations https://deprecations.emberjs.com/id/ember-component-send-action/ , sendAction is no longer supported. This PR removes it's dependency.