Skip to content

Feat: Disable unallowed combobox options for menubar and toolbar visibility#14

Open
avocado-lynx wants to merge 4 commits intoswe-productivity:developmentfrom
avocado-lynx:issue4
Open

Feat: Disable unallowed combobox options for menubar and toolbar visibility#14
avocado-lynx wants to merge 4 commits intoswe-productivity:developmentfrom
avocado-lynx:issue4

Conversation

@avocado-lynx
Copy link
Copy Markdown

Brief overview of PR changes/additions

This pull visually disables disallowed setting options so users can intuitively understand them being disallowed.
The previous behavior was to "reset" to an allowed configuration right after the user makes a disallowed change.

Other info (issues closed, discussion etc)

Fixes #4

@vadi2
Copy link
Copy Markdown
Collaborator

vadi2 commented Mar 20, 2026

Clean and intuitive UI approach to this, I like it!

One genuine issue: the silent early return at line 382-384 when qobject_cast fails. The same file uses qWarning() for identical cast failure patterns (lines 1082-1089 and 3971-3974), so this should too. If the cast ever fails, the entire visual-disabling feature silently stops working with no diagnostic output.

Two pre-existing issues worth fixing while you're here: the comment "force it back to the "Only if no profile one" is truncated (missing closing quote and rest of sentence) in both slot handlers, and the zero-check style is inconsistent on line 4041 where the left side uses explicit newIndex == 0 but the right side still uses implicit !comboBox_toolBarVisibility->currentIndex().

One simplification suggestion: the two-phase "reset both to enabled, then conditionally disable one" logic in updateVisibilityCombosConstraints could be replaced with two direct calls that compute the final state in one pass:

setComboItemEnabled(menuBarModel, 0, toolIndex != 0);
setComboItemEnabled(toolBarModel, 0, menuIndex != 0);

This is shorter and makes the invariant immediately obvious.

@avocado-lynx
Copy link
Copy Markdown
Author

I've pushed fixes for all of those issues. Does it look fine now?

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Deactivate option to set both toolbars to simultaneusly show "never"

2 participants