Preferences fixes by Be-ing · Pull Request #1377 · mixxxdj/mixxx

Be-ing · 2017-11-12T02:03:34Z

Split bottom sections of Interface preference pane into new Deck pane
Fix Interface and Deck panes so settings are only applied when pressing Apply button
Fix loading of rate range from empty profile https://bugs.launchpad.net/mixxx/+bug/1676140

Only did this for the top section so far. The rest will be split into a new Decks pane.

so that Apply button must be pressed for changed setting to take effect TODO: do this for the rest of the preferences in DlgPrefDeck

until Apply button is pressed

to prevent recurrence of bugs like https://bugs.launchpad.net/mixxx/+bug/1631393 when rateRange was 0

…alue until Apply button is pressed

until Apply button is pressed

Be-ing · 2017-11-12T08:55:42Z

Yikes there were a lot of dusty cobwebs in that code! I didn't take care of all of them, but it should be more manageable now. Please test and make sure nothing broke.

Copying @ywwg's checklist for testing preference changes:

Restore defaults should work
change some values, hit cancel, reenter pane. Changes should have been cancelled
change some values, hit apply, close and reenter pane. Changes should have been accepted
change some values, apply, see that Mixxx actually obeys the changes.
change some values, apply, exit mixxx, restart. Changes should still be reflected.

This whole checklist should be tested for every option in both the Interface and Decks panes.

Be-ing · 2017-11-12T09:12:04Z

    m_pControlsButton->setFlags(Qt::ItemIsSelectable | Qt::ItemIsEnabled);

+    m_pDecksButton = new QTreeWidgetItem(contentsTreeWidget, QTreeWidgetItem::Type);
+    m_pDecksButton->setIcon(0, QIcon(":/images/preferences/ic_preferences_decks.png"));


Note that this icon for the new Decks preference pane does not exist yet. @ronso0 or @esbrandt would you like to make it?

Something like a Play icon should do.

What's your preference? I like the boxed Play more because I asscociate it with a container rather than behaviour

You can take them from the comment, they are not altered/compressed.

ronso0 · 2017-11-12T16:38:43Z

@@ -0,0 +1,623 @@
+/***************************************************************************
+                           dlgprefcontrols.cpp  -  description


dlgprefdeck.cpp ?

Good catch, thanks. That comment could be removed entirely.

daschuer · 2017-11-12T19:23:34Z

If you change to auto scaling the default skin is loaded and not the configured skin.

How about moving the waveform options into root?

Be-ing · 2017-11-12T19:25:22Z

If you change to auto scaling the default skin is loaded and not the configured skin.

I'll look into this.

How about moving the waveform options into root?

Yep, I am planning on it.

sblaisot · 2017-11-13T08:53:34Z

Can you please provide before/after screenshots in UI related PRs? this way, one doesn't have to compile/install a version including this PR to see what changed.

ronso0 · 2017-11-15T00:29:36Z

Bug 1731774 could be fixed here as well.
Do you plan to do so? If yes, do we have vector sources for those images somewhere?
I can help with that, it's a matter of minutes once we agreed on an appropriate bg color.

Pegasus-RPG · 2017-11-15T21:13:30Z

I second the inclusion of before & after screen shots.

Be-ing · 2017-11-16T03:49:48Z

Not much has changed with the GUI. I'm mostly looking for people to test to make sure nothing broke.

Before:

After:

ronso0 · 2017-11-16T21:35:43Z

Looks nice!

Now that the Interface section is cleaned up, what about displaying the skin <description>
below skin drop-down like we have for controllers?

…ng Apply somehow DlgPrefInterface::slotApply is shown when closing the Preferences window even if the Interface pane is never shown and the Okay button is used to close the window. Also, configuration values should never be set outside of the preferences code!

Be-ing · 2017-11-20T00:05:32Z

            configSkin = getDefaultSkinName();
        }
-        m_pConfig->set(ConfigKey("[Config]", "ResizableSkin"),
-                       ConfigValue(configSkin));


It should not be possible to set configuration values outside of the preferences code! The whole preferences system could really use a thorough cleanup. It's very brittle.

This order is somewhat arbitrary. I am trying to put it in order of what I think is most important for users to configure first.

@ronso0

Thanks @ronso0 !

daschuer · 2017-11-26T21:08:28Z

The numeric value of the ramping pitch band slider is somehow meaningless.
It would be nice to set it to set the slider in relation to the fin an coarse values below.
Can we mark the slider ends reasonable?

daschuer · 2017-11-26T21:11:48Z

In 50% scaling is the Deere parallel waveforms are too high

Be-ing · 2017-12-18T04:02:44Z

Okay, I think this is done. Please test.

Be-ing · 2017-12-21T08:29:32Z

Ping. The rate range bug must be fixed for the beta release.

ronso0 · 2017-12-21T17:54:55Z

Anything specific to test?

Be-ing · 2017-12-21T19:01:50Z

Refer to the checklist above

ronso0 · 2017-12-21T19:20:26Z

I started without a config file.
In all categories I checked, individual settings persist.

Sound Hardware: works

Library: works

Controllers: enabled mapping for USB mouse, setting persists. Restore Defaults won't uncheck it though

AutoDJ: works

Decks: Restore Defaults doesn't work, they appear to be applied in Preferences but aren't in skin. When going back to Preferences all my individual settings are back. Same after restart.

Interface: same issue

Waveforms: initially waveform zoom was set to 33% (started with Shade). Restore Defaults sets this to 20%, but individual setting Synchronize Zoom is always re-checked when opening Prefs again

Equalizer: deck-specific EQ/QuickEffect settings don't persist a restart, instead this is reset and individual settings from deck1 are applied

Crossfader: works

Live Broadcasting: don't now if it's intended, "Turn on Live..." is reset to Off after disconnecting Broadcast in the menu. No further testing

Normalization: Apply ReplayGain is re-checked after restart, replay gain mode persists

Recording: defaults don't persist after resetting from individual settings

Beat Detection: works

Key Detection: works

Be-ing · 2017-12-21T22:06:28Z

Regarding restoring defaults, I was under the impression that clicking that button should not immediately apply the defaults and the Apply button should need to be pressed to get them to apply. @ronso0 are you saying that is not intuitive? Should Restore to Defaults also apply immediately?

ronso0 · 2017-12-21T22:14:47Z

No, Restore Defaults sets all options and Apply ..well applies them unless one hits Cancel.
I saw the check list just after reporting my results, and considering the results, I didn't repeat the tests without Apply ;)

Be-ing · 2017-12-22T06:11:57Z

So should we merge this as-is or should I make the Reset to Defaults buttons in the Interface and Decks panes immediately apply the defaults?

daschuer · 2017-12-22T06:17:29Z

Reset do defaults should not imply Apply. A user may whish to just check the default value for a few items without applying all.

How is the state of my findings? I think we need a Launchpad bugs if they are not addressed here.

Be-ing · 2017-12-22T06:55:52Z

If I change the Cue mode, from Pioneer to Mixxx, the play button keeps blinking. It stops blinking after using it.

I cannot reproduce this.

The numeric value of the ramping pitch band slider is somehow meaningless.
It would be nice to set it to set the slider in relation to the fin an coarse values below.
Can we mark the slider ends reasonable?

With what? "Low" and "High"? Which label belongs on which side?

In 50% scaling is the Deere parallel waveforms are too high

I'm not sure what you mean, but that doesn't sound like a bug with the preferences. It sounds like a bug in the waveforms or skin.

All other notes addressed.

daschuer · 2017-12-22T07:05:03Z

Ah okay, so the PR related things are done.
Good work, thank you!

Be-ing added 7 commits November 11, 2017 19:55

DlgPrefControls: remove "(Experimental)" label from skin scaling

72ef93c

DlgPrefControls: don't apply changes until Apply button is pressed

be6d495

Only did this for the top section so far. The rest will be split into a new Decks pane.

split Interface preference pane into Interface and Decks

04424a3

DlgPrefDeck: detangle rate range combobox from CO value

f2354f9

so that Apply button must be pressed for changed setting to take effect TODO: do this for the rest of the preferences in DlgPrefDeck

DlgPrefDeck: detangle rate inversion checkbox from CO

a63769f

until Apply button is pressed

code cleanup (use enum classes instead of magic ints)

6a257db

change "rateRange" CO to a ControlPotmeter to set limits on it

29ef9d1

to prevent recurrence of bugs like https://bugs.launchpad.net/mixxx/+bug/1631393 when rateRange was 0

Be-ing force-pushed the preferences_cleanup branch from 5f9a243 to 29ef9d1 Compare November 12, 2017 05:55

Be-ing added 5 commits November 12, 2017 00:33

DlgPrefDeck: detangle playing track protection checkbox from config v…

f413afa

…alue until Apply button is pressed

DlgPrefDeck: detangle load tracks to cue point checkbox from ConfigValue

d05ad94

until Apply button is pressed

DlgPrefDeck: detangle cue mode selection buttons from ConfigValue

18ce6d3

until Apply button is pressed

DlgPrefDeck: detangle rate adjustment widgets from ConfigValues

042ac2e

until Apply button is pressed

DlgPrefDeck: detangle track time setting from CO

dfaa7cf

until Apply button is pressed

Be-ing mentioned this pull request Nov 12, 2017

Pitch/Tempo slider: set default range for clean profile #1230

Closed

Be-ing commented Nov 12, 2017

View reviewed changes

ronso0 reviewed Nov 12, 2017

View reviewed changes

Be-ing commented Nov 20, 2017

View reviewed changes

Be-ing added 4 commits November 19, 2017 18:21

move Waveform preferences out to equal tree level as other prefs

c06a924

cleanup variable names in DlgPreferences

7b6c6bf

reorder preferences pages in list

d9c3ea0

This order is somewhat arbitrary. I am trying to put it in order of what I think is most important for users to configure first.

add icon for Decks Preferences

f10f00d

Thanks @ronso0 !

Be-ing added 6 commits November 29, 2017 23:48

Merge remote-tracking branch 'upstream/master' into preferences_cleanup

2d6d765

Merge remote-tracking branch 'upstream/master' into preferences_cleanup

92c55aa

remove unnecesary file heading comments

7e4aa9b

break up long lines of code

330473f

fix updating of track time display in preferences when loading dialog

2ea6e9a

make DlgPrefInterface Cancel button work

d89f84c

Be-ing mentioned this pull request Dec 19, 2017

Tango :: small fixes + Master/Head mixer toggle #1423

Merged

daschuer merged commit 15f84d7 into mixxxdj:master Dec 22, 2017

Be-ing mentioned this pull request Dec 22, 2017

move effects post-fader #1254

Merged

2 tasks

Be-ing deleted the preferences_cleanup branch December 22, 2017 07:13

This was referenced Dec 22, 2017

Responsive and thread-safe track analysis (ad-hoc + batch) #1413

Closed

fix Windows build #1437

Merged

Be-ing mentioned this pull request Feb 26, 2018

skin preview screenshots in Interface preferences #1525

Merged

		@@ -0,0 +1,623 @@
		/***************************************************************************
		dlgprefcontrols.cpp - description

Uh oh!

Conversation

Be-ing commented Nov 12, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Be-ing commented Nov 12, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Be-ing Nov 12, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ronso0 Nov 12, 2017

Choose a reason for hiding this comment

Uh oh!

ronso0 Nov 12, 2017

Choose a reason for hiding this comment

Uh oh!

ronso0 Nov 12, 2017

Choose a reason for hiding this comment

Uh oh!

ronso0 Nov 12, 2017

Choose a reason for hiding this comment

Uh oh!

Be-ing Nov 12, 2017

Choose a reason for hiding this comment

Uh oh!

daschuer Nov 26, 2017

Choose a reason for hiding this comment

Uh oh!

daschuer commented Nov 12, 2017

Uh oh!

Be-ing commented Nov 12, 2017

Uh oh!

sblaisot commented Nov 13, 2017

Uh oh!

ronso0 commented Nov 15, 2017

Uh oh!

Pegasus-RPG commented Nov 15, 2017

Uh oh!

Be-ing commented Nov 16, 2017

Uh oh!

ronso0 commented Nov 16, 2017

Uh oh!

Be-ing Nov 20, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

daschuer commented Nov 26, 2017

Uh oh!

daschuer commented Nov 26, 2017

Uh oh!

Be-ing commented Dec 18, 2017

Uh oh!

Be-ing commented Dec 21, 2017

Uh oh!

ronso0 commented Dec 21, 2017

Uh oh!

Be-ing commented Dec 21, 2017

Uh oh!

ronso0 commented Dec 21, 2017

Uh oh!

Be-ing commented Dec 21, 2017

Uh oh!

ronso0 commented Dec 21, 2017

Uh oh!

Be-ing commented Dec 22, 2017

Uh oh!

daschuer commented Dec 22, 2017

Uh oh!

Be-ing commented Dec 22, 2017

Uh oh!

daschuer commented Dec 22, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Be-ing commented Nov 12, 2017 •

edited

Loading

Be-ing commented Nov 12, 2017 •

edited

Loading

Be-ing Nov 12, 2017 •

edited

Loading

Be-ing Nov 20, 2017 •

edited

Loading