Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Ensure toolbar fits controls that are taller than the buttons in wxMS…
…W toolbar
  • Loading branch information
JulianSmart committed Nov 24, 2015
1 parent ed4dbc0 commit 0185d61
Showing 1 changed file with 31 additions and 1 deletion.
32 changes: 31 additions & 1 deletion src/msw/toolbar.cpp
Expand Up @@ -496,6 +496,20 @@ wxSize wxToolBar::DoGetBestSize() const

if ( !IsVertical() )
{
wxToolBarToolsList::compatibility_iterator node;
for ( node = m_tools.GetFirst(); node; node = node->GetNext() )
{
wxToolBarTool * const
tool = static_cast<wxToolBarTool *>(node->GetData());
if (tool->IsControl())
{
int y = tool->GetControl()->GetSize().y;
// Approximate border size
if (y > (sizeBest.y - 4))

This comment has been minimized.

Copy link
@vadz

vadz Nov 24, 2015

Contributor

It would be more clear to have a symbolic constant for this 4, especially as it's repeated below.

sizeBest.y = y + 4;
}
}

// Without the extra height, DoGetBestSize can report a size that's
// smaller than the actual window, causing windows to overlap slightly
// in some circumstances, leading to missing borders (especially noticeable
Expand Down Expand Up @@ -1090,7 +1104,9 @@ bool wxToolBar::Realize()
if ( diff < 0 )
{
// the control is too high, resize to fit
control->SetSize(wxDefaultCoord, height - 2);
// Actually don't set the size, otherwise we can never fit

This comment has been minimized.

Copy link
@vadz

vadz Nov 24, 2015

Contributor

This code and comment are really confusing now. I have no idea what was going on here even before this change, but keeping the old commented out code doesn't help.

Presumably this case (diff < 0) can't happen at all any more after this commit, so shouldn't we just remove all this if entirely?

// the toolbar around the controls.
// control->SetSize(wxDefaultCoord, height - 2);

diff = 2;
}
Expand Down Expand Up @@ -1733,6 +1749,20 @@ bool wxToolBar::HandleSize(WXWPARAM WXUNUSED(wParam), WXLPARAM lParam)
h = r.bottom - r.top - 3;
else
h = r.bottom - r.top;

// Take control height into account

This comment has been minimized.

Copy link
@vadz

vadz Nov 24, 2015

Contributor

It would be nice to avoid duplicating this code. As it is, we have the almost but not quite the same version of it in DoGetBestSize() which is not ideal.

for ( node = m_tools.GetFirst(); node; node = node->GetNext() )
{
wxToolBarTool * const
tool = static_cast<wxToolBarTool *>(node->GetData());
if (tool->IsControl())
{
int y = (tool->GetControl()->GetSize().y - 2); // -2 since otherwise control height + 4 (below) is too much
if (y > h)
h = y;
}
}

if ( m_maxRows )
{
// FIXME: hardcoded separator line height...
Expand Down

0 comments on commit 0185d61

Please sign in to comment.