always create [Library],maximize_library

[ronso0]

https://bugs.launchpad.net/mixxx/+bug/1941027

[Holzhaus]

Let's reduce redundancy if we rename it anyway.

Also, the active verb in maximize_library makes it sound like a button (e.g. [ChannelN],hotcue_X_activate), which means that you have to press [1.0] and release [0.0] to enable, the press and release again to disable.

But in this case it's actually a flag (1.0 = enabled, 0.0 = disabled). So I think we should use a passive verb here to indicate that (like we do in [ChannelN],loop_enabled).

Suggested change

	const ConfigKey kMaximizeLibraryConfigKey = ConfigKey("[Library]", "maximize_library");
	const ConfigKey kMaximizeLibraryConfigKey = ConfigKey("[Library]", "maximized");

[ronso0]

I see what you mean, but I think for consistency we should either name it like show_previewdeck and other visibility controls -- or rename all to [Skin],library_maximized, [Skin],previewdeck_visible etc.
Btw despite the redundancy the benefit of maximize_library is that the name of the control matches the tooltip which is convenient for skin development.

And actually the reason for using the [Library] is solely that it's listed in the respective chapter in the manual, see the bug report. [Skin],maxmize_library would be more appropriate IMO.
On that topic: I think we should rename the chapters to Deck controls, Skin controls etc., much like the submenus in the control picker menu. Also, the Group elements in the chapters do decrease the readability IMHO, so I'd appreciate if we'd rename them.

[ronso0]

rename the chapters to Deck controls, Skin controls etc., much like the submenus in the control picker menu.

vs.

[Be-ing]

I agree. Since we are making an alias anyway, let's call it [Library], maximized.

[ronso0]

Btw this approach is a regression right now because the menu action is enabled regardless if the skin actually supplies that control.

[ronso0]

@Holzhaus Thanks for your objection, it made me realize this approach would somehow solve the bug but the root cause is elsewhere.

[ronso0]

Closing this because I'm not keen on fixing/extending VisibilityConnection and the skin/qml parser to fix the regression.

[Be-ing]

Closing this because I'm not keen on fixing/extending VisibilityConnection and the skin/qml parser to fix the regression.

If integration with the menu bar is causing problems, let's just remove the menu bar entry. The menu bar's days are limited anyway.

[Be-ing]

@ronso0 do you want to finish this? If not, I will.

[ronso0]

If not, I will.

Go ahead!

[ronso0]

If integration with the menu bar is causing problems, let's just remove the menu bar entry.

A reason to keep the menubar button is that it makes Space = "maximize library" discoverable for users who don't use and therefore don't look for (deck) keyboard shortcuts in the PDF but would appreciate a shortcut for maximizing the library.

If you remove the action add the shortcut to the tooltip (and bind the shortcut in coreservices, of course)

mixxx/src/skin/legacy/tooltips.cpp

Lines 277 to 279 in df735e0

	add("maximize_library")
	<< tr("Maximize Library")
	<< tr("Hide all skin sections except the decks to have more screen space for the track library.");

[ronso0]

Since actually all visibility controls created by (official) skins are affected we should consider a different approach.
Let's discuss this on Zulip [PreviewDeck], show_previewdeck

[github-actions]

This PR is marked as stale because it has been open 90 days with no activity.

[ronso0]

Superseeded by #4647