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

Fix Preferences Buttons #2664

Merged
merged 10 commits into from
Apr 17, 2020
Merged

Conversation

Holzhaus
Copy link
Member

A bunch of pages don't implement slotCancel and slotResetToDefaults.

I implemented slotResetToDefaults for the colors and the controller pages and added a default slotCancel implementation that falls back to slotUpdate.

To prevent situations like this in the future, I deleted the default implementation for slotUpdate/slotApply/slotResetToDefault so that pages have to implement them.

For the LV2 page I added empty functions with a TODO comment for now, since I don't have any LV2 plugins and can't test it.

@Holzhaus Holzhaus added this to the 2.3.0 milestone Apr 14, 2020
@Holzhaus Holzhaus self-assigned this Apr 14, 2020
@@ -17,16 +17,18 @@ class DlgPrefColors : public DlgPreferencePage, public Ui::DlgPrefColorsDlg {
virtual ~DlgPrefColors();

public slots:
// Apply changes to widget
/// Called when the preference dialog (not this page) is shown to the user.
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we really need to copy documentation from parent classes into every derived class?

Copy link
Member Author

Choose a reason for hiding this comment

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

No, but it doesn't hurt either. I just replaced the already existing comments with better ones.

@Be-ing Be-ing merged commit f161e03 into mixxxdj:master Apr 17, 2020
@Be-ing
Copy link
Contributor

Be-ing commented Apr 17, 2020

Thank you for taking care of this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants