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

Fixed the problem that m_pCleanUpButton UI is displayed empty when NoIcons=1. #2273

Merged
merged 2 commits into from
Sep 25, 2022

Conversation

okrc
Copy link
Contributor

@okrc okrc commented Sep 25, 2022

Fixed the problem that m_pCleanUpButton UI is displayed empty when NoIcons=1.
image
image

@okrc okrc changed the title Update SandMan.cpp Fixed the problem that m_pCleanUpButton UI is displayed empty when NoIcons=1. Sep 25, 2022
Copy link
Collaborator

@isaak654 isaak654 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there should be a vertical separator between "Keep terminated" and "Cleanup".

@okrc
Copy link
Contributor Author

okrc commented Sep 25, 2022

I think there should be a vertical separator between "Keep terminated" and "Cleanup".

Added.

Copy link
Collaborator

@isaak654 isaak654 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is happening with W7 / W11 x64, and there is no top border in the first two buttons:
buttons_noicons

EDIT: it is a thing even in older Sandman builds.
Resolved by okrc: #2280

@DavidXanatos DavidXanatos merged commit 408280a into sandboxie-plus:master Sep 25, 2022
@isaak654
Copy link
Collaborator

Of course, my last review still stands (merging or not).

@DavidXanatos
Copy link
Member

I have commented out the separator, its intended that the vertical bars are not ever ware, rest of the changes are in and active

@isaak654
Copy link
Collaborator

I have commented out the separator, its intended that the vertical bars are not ever ware, rest of the changes are in and active

This might be fine, but the visual inconsistency with Win7 (which is still supported in SBIE at this time) is still there.

@okrc
Copy link
Contributor Author

okrc commented Sep 25, 2022

@isaak654 I zoomed in several times and found that this problem also exists on Windows 11, which is supposed to be caused by m_pMenuBar.

QWidget* pMenuWidget = new QWidget(this);
m_pMenuLayout = new QHBoxLayout(pMenuWidget);
m_pMenuLayout->setContentsMargins(0, 0, 0, 0);
m_pMenuLayout->addWidget(m_pMenuBar);
//m_pMenuLayout->addWidget(m_pLabel);
//m_pMenuLayout->addStretch(10);
setMenuWidget(pMenuWidget);

Commenting out this code should solve the problem, I'm not sure what the intent of this code is.
I tried to change it to

	QWidget* pMenuWidget = new QWidget(this);
	m_pMenuLayout = new QHBoxLayout(pMenuWidget);
	m_pMenuLayout->setContentsMargins(0, 0, 0, 0);
	//m_pMenuLayout->addWidget(m_pMenuBar);
	m_pMenuLayout->setMenuBar(m_pMenuBar);
	//m_pMenuLayout->addWidget(m_pLabel);
	//m_pMenuLayout->addStretch(10);
	setMenuWidget(pMenuWidget);

It seems to be solved.

@okrc okrc deleted the okrc-patch-1 branch September 25, 2022 10:02
@isaak654
Copy link
Collaborator

isaak654 commented Sep 26, 2022

@okrc
I can confirm that your latest modification works well in a separate test build.
If you're planning to create a new pull request with the m_pMenuBar fix, I'm going to merge it straight away.

@okrc
Copy link
Contributor Author

okrc commented Sep 26, 2022

@isaak654
I created #2280

isaak654 pushed a commit that referenced this pull request Sep 26, 2022
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.

3 participants