-
Notifications
You must be signed in to change notification settings - Fork 8.6k
DEV: move setting-component mixin logic into site-setting component #32958
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
Conversation
28bc09f
to
c151917
Compare
.uniq() | ||
.join("|"); | ||
this.setting.validationMessage = null; | ||
return false; |
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 return value is most likely not needed anymore (it used to work like a preventDefault() back in the day)
export default class SiteSettingComponent extends Component { | ||
@service modal; | ||
@service router; | ||
@service site; |
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.
@service site; |
I think this one is unused?
classNameBindings = [":row", ":setting", "overridden", "typeClass"]; | ||
attributeBindings = ["setting.setting:data-setting"]; | ||
|
||
_handleKeydown = async (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 this handler be a regular method with @bind
decorator?
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.
went with action
as it binds as well.
1f9cede
to
102b42f
Compare
Follow up to #32958. This removes the setting-component mixin which has been merged into the sole parent component still using it.
This PR moves the SettingComponent logic into SiteSetting component. The mixin itself will be removed in a separate PR.