diff --git a/docs/changes.txt b/docs/changes.txt index 3a288f2ca3..af29fa7cf9 100644 --- a/docs/changes.txt +++ b/docs/changes.txt @@ -65,6 +65,11 @@ Changes in behaviour not resulting in compilation errors - wxGTK wxNotebook::AddPage() doesn't generate any events any more for the first page being added, for consistency with the other ports. +- wxMSW wxToolBar height now adapts to the height of embedded controls, making + the toolbar taller if necessary, rather than making the controls smaller. To + return to the previous behaviour, you need to explicitly create controls of + smaller size. + Changes in behaviour which may result in build errors ----------------------------------------------------- diff --git a/include/wx/msw/toolbar.h b/include/wx/msw/toolbar.h index 5423f771f1..ba3581515b 100644 --- a/include/wx/msw/toolbar.h +++ b/include/wx/msw/toolbar.h @@ -174,6 +174,20 @@ private: WXHBRUSH MSWGetToolbarBgBrush(); #endif // wxHAS_MSW_BACKGROUND_ERASE_HOOK + // Return true if we're showing the labels for the embedded controls: we + // only do it if text is enabled and, somewhat less expectedly, if icons + // are enabled too because showing both the control and its label when only + // text is shown for the other buttons is too inconsistent to be useful. + bool AreControlLabelsShown() const + { + return HasFlag(wxTB_TEXT) && !HasFlag(wxTB_NOICONS); + } + + // Return the size required to accommodate the given tool which must be of + // "control" type. + wxSize MSWGetFittingtSizeForControl(class wxToolBarTool* tool) const; + + wxDECLARE_EVENT_TABLE(); wxDECLARE_DYNAMIC_CLASS(wxToolBar); wxDECLARE_NO_COPY_CLASS(wxToolBar); diff --git a/samples/toolbar/toolbar.cpp b/samples/toolbar/toolbar.cpp index 63ba471f6d..f787d739d7 100644 --- a/samples/toolbar/toolbar.cpp +++ b/samples/toolbar/toolbar.cpp @@ -350,7 +350,7 @@ void MyFrame::RecreateToolbar() style |= wxTB_RIGHT; break; case TOOLBAR_BOTTOM: - style |= wxTB_BOTTOM; + style |= wxTB_BOTTOM; break; } @@ -574,16 +574,16 @@ MyFrame::MyFrame(wxFrame* parent, tbarMenu->AppendSeparator(); tbarMenu->AppendRadioItem(IDM_TOOLBAR_TOP_ORIENTATION, - "Set toolbar at the top of the window", + "Set toolbar at the top of the window\tCtrl-Up", "Set toolbar at the top of the window"); tbarMenu->AppendRadioItem(IDM_TOOLBAR_LEFT_ORIENTATION, - "Set toolbar at the left of the window", + "Set toolbar at the left of the window\tCtrl-Left", "Set toolbar at the left of the window"); tbarMenu->AppendRadioItem(IDM_TOOLBAR_BOTTOM_ORIENTATION, - "Set toolbar at the bottom of the window", + "Set toolbar at the bottom of the window\tCtrl-Down", "Set toolbar at the bottom of the window"); tbarMenu->AppendRadioItem(IDM_TOOLBAR_RIGHT_ORIENTATION, - "Set toolbar at the right edge of the window", + "Set toolbar at the right edge of the window\tCtrl-Right", "Set toolbar at the right edge of the window"); tbarMenu->AppendSeparator(); diff --git a/src/msw/toolbar.cpp b/src/msw/toolbar.cpp index 3a397be743..901104ada5 100644 --- a/src/msw/toolbar.cpp +++ b/src/msw/toolbar.cpp @@ -94,6 +94,9 @@ #define TB_GETMAXSIZE (WM_USER + 83) #endif +// Margin between the control and its label. +static const int MARGIN_CONTROL_LABEL = 3; + // ---------------------------------------------------------------------------- // wxWin macros // ---------------------------------------------------------------------------- @@ -162,17 +165,7 @@ public: if ( IsControl() && !m_label.empty() ) { // Create a control to render the control's label. - // It has the same witdh as the control. - wxSize size(control->GetSize().GetWidth(), wxDefaultCoord); - m_staticText = new wxStaticText - ( - m_tbar, - wxID_ANY, - m_label, - wxDefaultPosition, - size, - wxALIGN_CENTRE | wxST_NO_AUTORESIZE - ); + m_staticText = new wxStaticText(m_tbar, wxID_ANY, m_label); } else // no label { @@ -475,6 +468,14 @@ void wxToolBar::Recreate() const wxPoint pos = GetPosition(); const wxSize size = GetSize(); + // Note that MSWCreateToolbar() will set the current size as the initial + // and minimal size of the toolbar, which is unwanted both because it loses + // any actual min size set from user code and because even if SetMinSize() + // is never called, we're going to be stuck with the bigger than necessary + // min size when we're switching from text+icons to text or icons-only + // modes, so preserve the current min size here. + const wxSize minSizeOrig = GetMinSize(); + // Hide the toolbar before recreating it to ensure that wxFrame doesn't try // to account for its size, e.g. to offset the position of the new toolbar // being created by the size of this toolbar itself. This wouldn't work @@ -493,6 +494,8 @@ void wxToolBar::Recreate() return; } + SetMinSize(minSizeOrig); + // Undo the effect of Hide() above. Show(); @@ -543,68 +546,101 @@ wxToolBar::~wxToolBar() delete m_disabledImgList; } +wxSize wxToolBar::MSWGetFittingtSizeForControl(wxToolBarTool* tool) const +{ + wxSize size = tool->GetControl()->GetBestSize(); + + // This is arbitrary, but we want to leave at least 1px around the control + // vertically, otherwise it really looks too cramped. + size.y += 2*1; + + // Account for the label, if any. + if ( wxStaticText * const staticText = tool->GetStaticText() ) + { + if ( AreControlLabelsShown() ) + { + const wxSize sizeLabel = staticText->GetSize(); + + if ( size.x < sizeLabel.x ) + size.x = sizeLabel.x; + + size.y += sizeLabel.y; + size.y += MARGIN_CONTROL_LABEL; + } + } + + // Also account for the tool padding value: note that we only have to add + // half of it to each tool, as the total amount of packing is the sum of + // the right margin of the previous tool and the left margin of the next + // one. + size.x += m_toolPacking / 2; + + return size; +} + wxSize wxToolBar::DoGetBestSize() const { + const wxSize sizeTool = GetToolSize(); + wxSize sizeBest; - - SIZE size; - if ( !::SendMessage(GetHwnd(), TB_GETMAXSIZE, 0, (LPARAM)&size) ) + if ( IsVertical() ) { - // maybe an old (< 0x400) Windows version? try to approximate the - // toolbar size ourselves - sizeBest = GetToolSize(); - sizeBest.y += 2 * ::GetSystemMetrics(SM_CYBORDER); // Add borders - sizeBest.x *= GetToolsCount(); + sizeBest.x = sizeTool.x + 2 * ::GetSystemMetrics(SM_CXBORDER); + } + else + { + sizeBest.y = sizeTool.y + 2 * ::GetSystemMetrics(SM_CYBORDER); + } + + wxToolBarToolsList::compatibility_iterator node; + for ( node = m_tools.GetFirst(); node; node = node->GetNext() ) + { + wxToolBarTool * const + tool = static_cast(node->GetData()); - // reverse horz and vertical components if necessary if ( IsVertical() ) { - wxSwap(sizeBest.x, sizeBest.y); - } - } - else // TB_GETMAXSIZE succeeded - { - // but it could still return an incorrect result due to what appears to - // be a bug in old comctl32.dll versions which don't handle controls in - // the toolbar correctly, so work around it (see SF patch 1902358) - if ( !IsVertical() && wxApp::GetComCtl32Version() < 600 ) - { - // calculate the toolbar width in alternative way - const RECT rcFirst = wxGetTBItemRect(GetHwnd(), 0); - const RECT rcLast = wxGetTBItemRect(GetHwnd(), GetToolsCount() - 1); - - const int widthAlt = rcLast.right - rcFirst.left; - if ( widthAlt > size.cx ) - size.cx = widthAlt; - } - - sizeBest.x = size.cx; - sizeBest.y = size.cy; - } - - if ( !IsVertical() ) - { - wxToolBarToolsList::compatibility_iterator node; - for ( node = m_tools.GetFirst(); node; node = node->GetNext() ) - { - wxToolBarTool * const - tool = static_cast(node->GetData()); - if (tool->IsControl()) + if ( !tool->IsControl() ) { - int y = tool->GetControl()->GetSize().y; - // Approximate border size - if (y > (sizeBest.y - 4)) - sizeBest.y = y + 4; + sizeBest.y += sizeTool.y + m_toolPacking; } + //else: Controls are not shown in vertical toolbars at all. } + else + { + if ( tool->IsControl() ) + { + const wxSize sizeControl = MSWGetFittingtSizeForControl(tool); - // 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 - // in AUI layouts). - if (!(GetWindowStyle() & wxTB_NODIVIDER)) - sizeBest.y += 2; - sizeBest.y ++; + // Ensure we're tall enough for the embedded controls. + sizeBest.IncTo(wxSize(-1, sizeControl.y)); + + sizeBest.x += sizeControl.x; + } + else + { + sizeBest.x += sizeTool.x; + } + + // As explained in MSWGetFittingtSizeForControl() above, the actual + // margin used for a single tool is one half of the total packing. + sizeBest.x += m_toolPacking / 2; + } + } + + // Note that this needs to be done after the loop to account for controls + // too high to fit into the toolbar without the border size but that could + // fit if we had added the border beforehand. + if ( !HasFlag(wxTB_NODIVIDER) ) + { + if ( IsVertical() ) + { + sizeBest.x += 2 * ::GetSystemMetrics(SM_CXBORDER); + } + else + { + sizeBest.y += 2 * ::GetSystemMetrics(SM_CYBORDER); + } } return sizeBest; @@ -751,6 +787,8 @@ bool wxToolBar::Realize() if ( !wxToolBarBase::Realize() ) return false; + InvalidateBestSize(); + const size_t nTools = GetToolsCount(); // don't change the values of these constants, they can be set from the @@ -1061,21 +1099,9 @@ bool wxToolBar::Realize() switch ( tool->GetStyle() ) { case wxTOOL_STYLE_CONTROL: - if ( wxStaticText *staticText = tool->GetStaticText() ) + if ( !IsVertical() ) { - // Display control and its label only if buttons have icons - // and texts as otherwise there is not enough room on the - // toolbar to fit the label. - staticText-> - Show(HasFlag(wxTB_TEXT) && !HasFlag(wxTB_NOICONS)); - } - - // Set separator width/height to fit the control width/height - // taking into account tool padding value. - // (height is not used but it is set for the sake of consistency). - { - const wxSize sizeControl = tool->GetControl()->GetSize(); - button.iBitmap = m_toolPacking + (IsVertical() ? sizeControl.y : sizeControl.x); + button.iBitmap = MSWGetFittingtSizeForControl(tool).x; } wxFALLTHROUGH; @@ -1191,6 +1217,15 @@ bool wxToolBar::Realize() break; } + if ( IsVertical() ) + { + // MSDN says that TBSTATE_WRAP should be used for all buttons in + // vertical toolbars, so do it even if it doesn't seem to actually + // change anything in practice (including the problem with + // TB_AUTOSIZE mentioned in UpdateSize()). + button.fsState |= TBSTATE_WRAP; + } + lastWasRadio = isRadio; i++; @@ -1205,6 +1240,17 @@ bool wxToolBar::Realize() // Adjust controls and stretchable spaces // -------------------------------------- + // We don't trust the height returned by wxGetTBItemRect() as it may not + // have been updated yet, use the height that the toolbar will actually + // have instead. + int height = GetBestSize().y; + if ( !HasFlag(wxTB_NODIVIDER) ) + { + // We want just the usable height, so remove the space taken by the + // border/divider. + height -= 2 * ::GetSystemMetrics(SM_CYBORDER); + } + // adjust the controls size to fit nicely in the toolbar and compute its // total size while doing it m_totalFixedSize = 0; @@ -1217,10 +1263,15 @@ bool wxToolBar::Realize() if ( !tool->IsControl() ) { - if ( IsVertical() ) - m_totalFixedSize += r.bottom - r.top; - else - m_totalFixedSize += r.right - r.left; + // Stretchable space don't have any fixed size and their current + // size shouldn't count at all. + if ( !tool->IsStretchableSpace() ) + { + if ( IsVertical() ) + m_totalFixedSize += r.bottom - r.top; + else + m_totalFixedSize += r.right - r.left; + } continue; } @@ -1232,57 +1283,52 @@ bool wxToolBar::Realize() // good and wxGTK doesn't do it neither (and the code below can't // deal with this case) control->Hide(); + if ( wxStaticText * const staticText = tool->GetStaticText() ) + staticText->Hide(); continue; } control->Show(); - wxStaticText * const staticText = tool->GetStaticText(); - wxSize size = control->GetSize(); - wxSize staticTextSize; - if ( staticText && staticText->IsShown() ) + const wxSize controlSize = control->GetSize(); + + // Take also into account tool padding value: the amount of padding + // used for each tool is half of m_toolPacking, so the margin on each + // side is a half of that. + const int x = r.left + m_toolPacking / 4; + + // Greater of control and its label widths. + int totalWidth = controlSize.x; + + // Height of control and its label, if any, including the margin + // between them. + int totalHeight = controlSize.y; + + if ( wxStaticText * const staticText = tool->GetStaticText() ) { - staticTextSize = staticText->GetSize(); - staticTextSize.y += 3; // margin between control and its label - } + const bool shown = AreControlLabelsShown(); + staticText->Show(shown); - // position the control itself correctly vertically centering it on the - // icon area of the toolbar - int height = r.bottom - r.top - staticTextSize.y; - - int diff = height - size.y; - if ( diff < 0 || !HasFlag(wxTB_TEXT) ) - { - // not enough room for the static text - if ( staticText ) - staticText->Hide(); - - // recalculate height & diff without the staticText control - height = r.bottom - r.top; - diff = height - size.y; - if ( diff < 0 ) + if ( shown ) { - // the control is too high, resize to fit - // Actually don't set the size, otherwise we can never fit - // the toolbar around the controls. - // control->SetSize(wxDefaultCoord, height - 2); + const wxSize staticTextSize = staticText->GetSize(); - diff = 2; + if ( staticTextSize.x > totalWidth ) + totalWidth = staticTextSize.x; + + // Center the static text horizontally for consistency with the + // button labels and position it below the control vertically. + staticText->Move(x + (totalWidth - staticTextSize.x)/2, + r.top + (height + controlSize.y + - staticTextSize.y + + MARGIN_CONTROL_LABEL)/2); + + totalHeight += staticTextSize.y + MARGIN_CONTROL_LABEL; } } - else // enough space for both the control and the label - { - if ( staticText ) - staticText->Show(); - } - // Take also into account tool padding value. - control->Move(r.left + m_toolPacking/2, r.top + (diff + 1) / 2); - if ( staticText ) - { - staticText->Move(r.left + m_toolPacking/2 + (size.x - staticTextSize.x)/2, - r.bottom - staticTextSize.y); - } + control->Move(x + (totalWidth - controlSize.x)/2, + r.top + (height - totalHeight)/2); m_totalFixedSize += r.right - r.left; } @@ -1294,8 +1340,16 @@ bool wxToolBar::Realize() if ( !IsVertical() ) { if ( m_maxRows == 0 ) + { // if not set yet, only one row SetRows(1); + } + else + { + // In all the other cases, UpdateSize() is called by SetRows(), but + // when we don't call it here, call it directly instead. + UpdateSize(); + } } else if ( m_nButtons > 0 ) // vertical non empty toolbar { @@ -1304,32 +1358,6 @@ bool wxToolBar::Realize() SetRows(m_nButtons); } - InvalidateBestSize(); - UpdateSize(); - - if ( IsVertical() ) - { - // For vertical toolbar heights of buttons are incorrect - // unless TB_AUTOSIZE in invoked. - // We need to recalculate fixed elements size again. - m_totalFixedSize = 0; - toolIndex = 0; - for ( node = m_tools.GetFirst(); node; node = node->GetNext(), toolIndex++ ) - { - wxToolBarTool * const tool = (wxToolBarTool*)node->GetData(); - if ( !tool->IsStretchableSpace() ) - { - const RECT r = wxGetTBItemRect(GetHwnd(), toolIndex); - if ( !IsVertical() ) - m_totalFixedSize += r.right - r.left; - else if ( !tool->IsControl() ) - m_totalFixedSize += r.bottom - r.top; - } - } - // Enforce invoking UpdateStretchableSpacersSize() with correct value of fixed elements size. - UpdateSize(); - } - return true; } @@ -1404,31 +1432,20 @@ void wxToolBar::UpdateStretchableSpacersSize() const int newSize = --numSpaces ? sizeSpacer : sizeLastSpacer; if ( newSize != oldSize) { - if ( !::SendMessage(GetHwnd(), TB_DELETEBUTTON, toolIndex, 0) ) + WinStruct tbbi; + tbbi.dwMask = TBIF_BYINDEX | TBIF_SIZE; + tbbi.cx = newSize; + if ( !::SendMessage(GetHwnd(), TB_SETBUTTONINFO, + toolIndex, (LPARAM)&tbbi) ) { - wxLogLastError(wxT("TB_DELETEBUTTON (separator)")); + wxLogLastError(wxT("TB_SETBUTTONINFO (separator)")); } else { - TBBUTTON button; - wxZeroMemory(button); - - button.idCommand = tool->GetId(); - button.iBitmap = newSize; // set separator width/height - button.fsState = TBSTATE_ENABLED; - button.fsStyle = TBSTYLE_SEP; - if ( IsVertical() ) - button.fsState |= TBSTATE_WRAP; - if ( !::SendMessage(GetHwnd(), TB_INSERTBUTTON, toolIndex, (LPARAM)&button) ) - { - wxLogLastError(wxT("TB_INSERTBUTTON (separator)")); - } - else - { - // We successfully replaced this seprator, move all the controls after it - // by the corresponding amount (may be positive or negative) - offset += newSize - oldSize; - } + // We successfully updated the separator width, move all the + // controls appearing after it by the corresponding amount + // (which may be positive or negative) + offset += newSize - oldSize; } } @@ -1638,14 +1655,25 @@ void wxToolBar::SetRows(int nRows) const bool enable = (!IsVertical() && m_maxRows == 1) || (IsVertical() && (size_t)m_maxRows == m_nButtons); - const LPARAM state = MAKELONG(enable ? TBSTATE_ENABLED : TBSTATE_HIDDEN, 0); + LPARAM state = enable ? TBSTATE_ENABLED : TBSTATE_HIDDEN; + + if ( IsVertical() ) + { + // As in Realize(), ensure that TBSTATE_WRAP is used for all the + // tools, including separators, in vertical toolbar, and here it does + // make a difference: without it, the following tools wouldn't be + // visible because they would be on the same row as the separator. + state |= TBSTATE_WRAP; + } + wxToolBarToolsList::compatibility_iterator node; for ( node = m_tools.GetFirst(); node; node = node->GetNext() ) { wxToolBarTool * const tool = (wxToolBarTool*)node->GetData(); if ( tool->IsStretchableSpace() ) { - if ( !::SendMessage(GetHwnd(), TB_SETSTATE, tool->GetId(), state) ) + if ( !::SendMessage(GetHwnd(), TB_SETSTATE, + tool->GetId(), MAKELONG(state, 0)) ) { wxLogLastError(wxT("TB_SETSTATE (stretchable spacer)")); } @@ -1681,10 +1709,13 @@ wxToolBarToolBase *wxToolBar::FindToolForPosition(wxCoord x, wxCoord y) const void wxToolBar::UpdateSize() { - wxPoint pos = GetPosition(); - ::SendMessage(GetHwnd(), TB_AUTOSIZE, 0, 0); - if (pos != GetPosition()) - Move(pos); + // We used to use TB_AUTOSIZE here, but it didn't work at all for vertical + // toolbars and was more trouble than it was worth for horizontal one as it + // added some unwanted margins that we had to remove later. So now we just + // compute our own size and use it. + SetSize(GetBestSize()); + + UpdateStretchableSpacersSize(); // In case Realize is called after the initial display (IOW the programmer // may have rebuilt the toolbar) give the frame the option of resizing the @@ -1867,116 +1898,12 @@ void wxToolBar::OnEraseBackground(wxEraseEvent& event) #endif // wxHAS_MSW_BACKGROUND_ERASE_HOOK } -bool wxToolBar::HandleSize(WXWPARAM WXUNUSED(wParam), WXLPARAM lParam) +bool wxToolBar::HandleSize(WXWPARAM WXUNUSED(wParam), WXLPARAM WXUNUSED(lParam)) { // wait until we have some tools - const int toolsCount = GetToolsCount(); - if ( toolsCount == 0 ) + if ( !GetToolsCount() ) return false; - // calculate our minor dimension ourselves - we're confusing the standard - // logic (TB_AUTOSIZE) with our horizontal toolbars and other hacks - // Find bounding box for all rows. - RECT r; - ::SetRectEmpty(&r); - // Bounding box for single (current) row - RECT rcRow; - ::SetRectEmpty(&rcRow); - int rowPosX = INT_MIN; - wxToolBarToolsList::compatibility_iterator node; - int i = 0; - for ( node = m_tools.GetFirst(); node; node = node->GetNext() ) - { - wxToolBarTool * const - tool = static_cast(node->GetData()); - if ( tool->IsToBeDeleted() ) - continue; - - // Skip hidden buttons - const RECT rcItem = wxGetTBItemRect(GetHwnd(), i); - if ( ::IsRectEmpty(&rcItem) ) - { - i++; - continue; - } - - if ( rcItem.top > rowPosX ) - { - // We have the next row. - rowPosX = rcItem.top; - - // Shift origin to (0, 0) to make it the same as for the total rect. - ::OffsetRect(&rcRow, -rcRow.left, -rcRow.top); - - // And update the bounding box for all rows. - ::UnionRect(&r, &r, &rcRow); - - // Reset the current row bounding box for the next row. - ::SetRectEmpty(&rcRow); - } - - // Separators shouldn't be taken into account as they are sometimes - // reported to have the width of the entire client area by the toolbar. - // And we know that they are not the biggest items in the toolbar in - // any case, so just skip them. - if( !tool->IsSeparator() ) - { - // Update bounding box of current row - ::UnionRect(&rcRow, &rcRow, &rcItem); - } - - i++; - } - - // Take into account the last row rectangle too. - ::OffsetRect(&rcRow, -rcRow.left, -rcRow.top); - ::UnionRect(&r, &r, &rcRow); - - if ( !r.right ) - return false; - - int w, h; - - if ( IsVertical() ) - { - w = r.right - r.left; - h = HIWORD(lParam); - } - else - { - w = LOWORD(lParam); - if (HasFlag( wxTB_FLAT )) - h = r.bottom - r.top - 3; - else - h = r.bottom - r.top; - - // Take control height into account - for ( node = m_tools.GetFirst(); node; node = node->GetNext() ) - { - wxToolBarTool * const - tool = static_cast(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... - h += HasFlag(wxTB_NODIVIDER) ? 4 : 6; - h *= m_maxRows; - } - } - - if ( MAKELPARAM(w, h) != lParam ) - { - // size really changed - SetSize(w, h); - } - UpdateStretchableSpacersSize(); // message processed