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 #2572: Fix invisible Quickchat bar on startup #2573

Merged
merged 1 commit into from Oct 5, 2016

Conversation

@Kissaki
Copy link
Member

commented Oct 5, 2016

Since 4009ea3 on Windows the quickchat
bar was invisible on startup due to height 0. We keep the new logic
introduced for other OSes, but for Windows, we keep the setupView(false)
call.

@mkrautz You wrote the change was tested on Windows. Did you not notice this problem?
PTAL

@Kissaki Kissaki added ui bug labels Oct 5, 2016

@Kissaki Kissaki added this to the 1.3.0 milestone Oct 5, 2016

@mkrautz
mkrautz approved these changes Oct 5, 2016
Copy link
Member

left a comment

Looks good to me. Feel free to use my snippet if you feel like it's a good idea.

@@ -333,7 +333,7 @@ void MainWindow::setupGui() {

updateTransmitModeComboBox();

#if QT_VERSION < 0x050000 && !defined(Q_OS_MAC)
#if defined(Q_OS_WIN) || (QT_VERSION < 0x050000 && !defined(Q_OS_MAC))

This comment has been minimized.

Copy link
@mkrautz

mkrautz Oct 5, 2016

Member

I guess that this should technically be QT_VERSION >= 0x050000 && defined(Q_OS_WIN)?

How about:

// For Qt >= 5, enable this call for Windows.
// For Qt < 5, enable for anything but macOS.
#if (QT_VERSION >= 0x050000 && defined(Q_OS_WIN) || (QT_VERSION < 0x050000 && !defined(Q_OS_MAC))

?

Only a technicality... I don't believe we actually support Qt 4 on Windows for 1.3.x.

This comment has been minimized.

Copy link
@Kissaki

Kissaki Oct 5, 2016

Author Member

I don’t mind either way. Mine was simpler/shorter, yours is more stuff but maybe more consistent in its two cases.

I changed to your snipped (plus missing parenthesis) and will land it.

@mkrautz

This comment has been minimized.

Copy link
Member

commented Oct 5, 2016

@Kissaki

I didn't notice it.

I mainly checked that the the mainwindow didn't do any crazy things (got hidden, etc.). It's very possible I missed the fact that the quick chatbar was gone or resized wrongly.

@Kissaki Kissaki force-pushed the Kissaki:fix-quickchatbar branch from e000c50 to 88a0f38 Oct 5, 2016

Fix #2572: Fix invisible Quickchat bar on startup
Since 4009ea3 on Windows the quickchat
bar was invisible on startup due to height 0. We keep the new logic
introduced for other OSes, but for Windows, we keep the setupView(false)
call.

@Kissaki Kissaki force-pushed the Kissaki:fix-quickchatbar branch from 88a0f38 to 9d2bb10 Oct 5, 2016

@Kissaki Kissaki merged commit f284052 into mumble-voip:master Oct 5, 2016

@Kissaki Kissaki deleted the Kissaki:fix-quickchatbar branch Oct 16, 2016

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