Skip to content

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

Merged
merged 4 commits into from
May 30, 2025

Conversation

tyb-talks
Copy link
Contributor

@tyb-talks tyb-talks commented May 28, 2025

This PR moves the SettingComponent logic into SiteSetting component. The mixin itself will be removed in a separate PR.

@tyb-talks tyb-talks force-pushed the dev-refactor-setting-component-mixin branch from 28bc09f to c151917 Compare May 28, 2025 08:03
@tyb-talks tyb-talks marked this pull request as ready for review May 28, 2025 09:44
.uniq()
.join("|");
this.setting.validationMessage = null;
return false;
Copy link
Contributor

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;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
@service site;

I think this one is unused?

classNameBindings = [":row", ":setting", "overridden", "typeClass"];
attributeBindings = ["setting.setting:data-setting"];

_handleKeydown = async (event) => {
Copy link
Contributor

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?

Copy link
Contributor Author

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.

@tyb-talks tyb-talks force-pushed the dev-refactor-setting-component-mixin branch from 1f9cede to 102b42f Compare May 30, 2025 06:09
@tyb-talks tyb-talks merged commit 27c7fcd into main May 30, 2025
16 checks passed
@tyb-talks tyb-talks deleted the dev-refactor-setting-component-mixin branch May 30, 2025 13:43
tyb-talks added a commit that referenced this pull request May 30, 2025
Follow up to #32958.

This removes the setting-component mixin which has been merged into the
sole parent component still using it.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants