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

Introduce MUComboBox subclass and use it throughout the tree. #2497

Merged
merged 4 commits into from Aug 8, 2016

Conversation

@mkrautz
Copy link
Member

commented Aug 7, 2016

This PR introduces MUComboBox and uses it throughout the tree.

Per the commit introducing MUComboBox:

Add MUComboBox.

This adds a new QComboBox subclass called MUComboBox
to widgets/MUComboBox.{cpp,h}.

This QComboBox subclass explicitly uses a QListView
as the backing view for the QComboBox.

This fixes various styling issues for QComboBox
on macOS.

By default on macOS, QComboBoxes are backed by
something that tries to emulate a native macOS
menu. However, that QAbstractItemView behaves
inconsistently when styled. For example, it does
not seem possible to set the size of individual
items, because they're restricted to the height
of a normal macOS menu item.
Also, in some cases (such as the QComboBox used
for the transmission picker in MainWindow's
QToolbar), the height of the QAbstractItemView
was also wrong when styled. This caused the combo
box to always scroll, even though it seemingly
was sized correctly.

To get consistent behavior, we use QListView as
the QComboBox's view in MUComboBox.

Also, at least for the default Mumble themes,
I've observed some weird eliding behavior for
the text of added items in the transmission picker
QComboBox in MainWindow's toolbar. Because of that,
for MUComboBox, we don't show ellipses by default.
This fixes the display of the combo box in the
MainWindow's toolbar.

Before:

image

After:

image

Before (with Mumble Dark, in ConfigDialog):

image

After (with Mumble Dark, in ConfigDialog):

image

Before (No theme, in ConfigDialog):

image

After (No theme, in ConfigDialog):

image

mkrautz added 4 commits Aug 7, 2016
Add MUComboBox.
This adds a new QComboBox subclass called MUComboBox
to widgets/MUComboBox.{cpp,h}.

This QComboBox subclass explicitly uses a QListView
as the backing view for the QComboBox.

This fixes various styling issues for QComboBox
on macOS.

By default on macOS, QComboBoxes are backed by
something that tries to emulate a native macOS
menu. However, that QAbstractItemView behaves
inconsistently when styled. For example, it does
not seem possible to set the size of individual
items, because they're restricted to the height
of a normal macOS menu item.
Also, in some cases (such as the QComboBox used
for the transmission picker in MainWindow's
QToolbar),  the height of the QAbstractItemView
was also wrong when styled. This caused the combo
box to always scroll, even though it seemingly
was sized correctly.

To get consistent behavior, we use QListView as
the QComboBox's view in MUComboBox.

Also, at least for the default Mumble themes,
I've observed some weird eliding behavior for
the text of added items in the transmission picker
QComboBox in MainWindow's toolbar. Because of that,
for MUComboBox, we don't show ellipses by default.
This fixes the display of the combo box in the
MainWindow's toolbar.
@mkrautz

This comment has been minimized.

Copy link
Member Author

commented Aug 7, 2016

Related issue: #2232

@mkrautz

This comment has been minimized.

Copy link
Member Author

commented Aug 7, 2016

Haven't yet tested the visual differences on Windows or X11.

@davidebeatrici davidebeatrici added the theme label Aug 7, 2016

@mkrautz mkrautz removed the theme label Aug 7, 2016

@mkrautz

This comment has been minimized.

Copy link
Member Author

commented Aug 7, 2016

Windows before:

no-combobox

Windows after:

mucombobox

Also an improvement there.

@hacst

This comment has been minimized.

Copy link
Member

commented Aug 7, 2016

Definitely looks like an improvement. LGTM.

@mkrautz mkrautz merged commit 66d41ef into mumble-voip:master Aug 8, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.