From 35c033d3faab986c3075aab1e08d6ac845b5b2c1 Mon Sep 17 00:00:00 2001 From: James Crook Date: Thu, 11 Aug 2016 21:54:45 +0100 Subject: [PATCH] Bug 1451 - On Mac the background of the Pinned/Unpinned button is not the same color as the Timeline Per comments in the bug, fixed by making the pinned/unpinned button more clearly a button. This involved adding a new type of grabber that does not have the ribs for dragging it and acts as a spacer. Also fixing grabber so that it does not have to be at position (0,0) Also making the ruler 1 pixel higher. Also changing the pin button to be a toggle button that changes from up to down on a click. Also fixing AButton so that an image can be bigger than the button. --- src/toolbars/ToolBar.cpp | 10 ++++++---- src/widgets/Grabber.cpp | 13 +++++++++++++ src/widgets/Grabber.h | 2 ++ src/widgets/Ruler.cpp | 26 ++++++++++++++++++++------ 4 files changed, 41 insertions(+), 10 deletions(-) diff --git a/src/toolbars/ToolBar.cpp b/src/toolbars/ToolBar.cpp index 63615c9a6..a01c41ea7 100644 --- a/src/toolbars/ToolBar.cpp +++ b/src/toolbars/ToolBar.cpp @@ -748,8 +748,9 @@ AButton * ToolBar::MakeButton(wxWindow *parent, bool processdownevents, wxSize size) { - int xoff = (size.GetWidth() - theTheme.Image(eStandardUp).GetWidth())/2; - int yoff = (size.GetHeight() - theTheme.Image(eStandardUp).GetHeight())/2; + // wxMax to cater for case of image being bigger than the button. + int xoff = wxMax( 0, (size.GetWidth() - theTheme.Image(eStandardUp).GetWidth())/2); + int yoff = wxMax( 0, (size.GetHeight() - theTheme.Image(eStandardUp).GetHeight())/2); typedef std::unique_ptr wxImagePtr; wxImagePtr up2 (OverlayImage(eUp, eStandardUp, xoff, yoff)); @@ -774,8 +775,9 @@ void ToolBar::MakeAlternateImages(AButton &button, int idx, teBmps eDisabled, wxSize size) { - int xoff = (size.GetWidth() - theTheme.Image(eStandardUp).GetWidth())/2; - int yoff = (size.GetHeight() - theTheme.Image(eStandardUp).GetHeight())/2; + // wxMax to cater for case of image being bigger than the button. + int xoff = wxMax( 0, (size.GetWidth() - theTheme.Image(eStandardUp).GetWidth())/2); + int yoff = wxMax( 0, (size.GetHeight() - theTheme.Image(eStandardUp).GetHeight())/2); typedef std::unique_ptr wxImagePtr; wxImagePtr up (OverlayImage(eUp, eStandardUp, xoff, yoff)); diff --git a/src/widgets/Grabber.cpp b/src/widgets/Grabber.cpp index d2c9b55f7..95467acc2 100644 --- a/src/widgets/Grabber.cpp +++ b/src/widgets/Grabber.cpp @@ -59,6 +59,7 @@ Grabber::Grabber(wxWindow * parent, wxWindowID id) { mOver = false; mPressed = false; + mAsSpacer = false; /* i18n-hint: A 'Grabber' is a region you can click and drag on It's used to drag a track around (when in multi-tool mode) rather @@ -99,6 +100,10 @@ void Grabber::SendEvent(wxEventType type, const wxPoint & pos, bool escaping) void Grabber::DrawGrabber( wxDC & dc ) { wxRect r = GetRect(); + // PaintDC positions are relative to the grabber, not the parent window. + // So use 0,0 as origin for draw, so that the grabber draws right if + // positioned in its parent at some non zero position. + r.SetPosition( wxPoint(0,0) ); int y, left, right, top, bottom; #ifndef EXPERIMENTAL_THEMING @@ -132,6 +137,9 @@ void Grabber::DrawGrabber( wxDC & dc ) #endif + // No bumps in a spacer grabber. + if( mAsSpacer ) + return; // Calculate the bump rectangle r.Deflate(3, 3); if ((r.GetHeight() % 4) < 2) { @@ -174,6 +182,8 @@ void Grabber::DrawGrabber( wxDC & dc ) // void Grabber::PushButton(bool state ) { + if( mAsSpacer ) + return; wxRect r = GetRect(); mOver = r.Contains(ScreenToClient(wxGetMousePosition())); @@ -207,6 +217,9 @@ void Grabber::OnEnter(wxMouseEvent & WXUNUSED(event)) UnsetToolTip(); SetToolTip(text); + if( mAsSpacer ) + return; + // Redraw highlighted mOver = true; Refresh(false); diff --git a/src/widgets/Grabber.h b/src/widgets/Grabber.h index a579e8dba..7f1f745aa 100644 --- a/src/widgets/Grabber.h +++ b/src/widgets/Grabber.h @@ -111,6 +111,7 @@ class Grabber final : public wxWindow bool AcceptsFocusFromKeyboard() const override {return false;} void PushButton(bool state); + void SetAsSpacer( bool bIsSpacer ) { mAsSpacer = bIsSpacer;}; protected: @@ -127,6 +128,7 @@ class Grabber final : public wxWindow bool mOver; bool mPressed; + bool mAsSpacer; public: diff --git a/src/widgets/Ruler.cpp b/src/widgets/Ruler.cpp index a832d8c1c..abf4c73f5 100644 --- a/src/widgets/Ruler.cpp +++ b/src/widgets/Ruler.cpp @@ -85,6 +85,7 @@ array of Ruler::Label. #include "../tracks/ui/Scrubbing.h" #include "../prefs/PlaybackPrefs.h" #include "../prefs/TracksPrefs.h" +#include "../widgets/Grabber.h" //#define SCRUB_ABOVE @@ -1656,14 +1657,14 @@ enum : int { TopMargin = 1, BottomMargin = 2, // for bottom bevel and bottom line - LeftMargin = 1, + LeftMargin = 1, RightMargin = 1, }; enum { ScrubHeight = 14, - ProperRulerHeight = 28 + ProperRulerHeight = 29 }; inline int IndicatorHeightForWidth(int width) @@ -2056,10 +2057,20 @@ void AdornedRulerPanel::ReCreateButtons() button = nullptr; } + size_t iButton = 0; // Make the short row of time ruler pushbottons. // Don't bother with sizers. Their sizes and positions are fixed. - wxPoint position{ 1 + LeftMargin, 0 }; - size_t iButton = 0; + // Add a grabber converted to a spacer. + // This makes it visually clearer that the button is a button. + + wxPoint position( 1, 0 ); + Grabber * pGrabber = safenew Grabber(this, this->GetId()); + pGrabber->SetAsSpacer( true ); + //pGrabber->SetSize( 10, 27 ); // default is 10,27 + pGrabber->SetPosition( position ); + + position.x = 12; + auto size = theTheme.ImageSize( bmpRecoloredUpSmall ); size.y = std::min(size.y, GetRulerHeight(false)); @@ -2078,7 +2089,7 @@ void AdornedRulerPanel::ReCreateButtons() mButtons[iButton++] = button; return button; }; - auto button = buttonMaker(OnTogglePinnedStateID, bmpPinnedPlayHead, false); + auto button = buttonMaker(OnTogglePinnedStateID, bmpPinnedPlayHead, true); ToolBar::MakeAlternateImages( *button, 1, bmpRecoloredUpSmall, bmpRecoloredDownSmall, bmpRecoloredHiliteSmall, @@ -2809,7 +2820,10 @@ void AdornedRulerPanel::UpdateButtonStates() { bool state = TracksPrefs::GetPinnedHeadPreference(); auto pinButton = static_cast(FindWindow(OnTogglePinnedStateID)); - pinButton->PopUp(); + if( !state ) + pinButton->PopUp(); + else + pinButton->PushDown(); pinButton->SetAlternateIdx(state ? 0 : 1); const auto label = state // Label descibes the present state, not what the click does