Bug 2357 ? - Non-standard behavior when right-clicking on pinned playhead/recordhead

This fix fixes a number of issues with right context menus and the pinned button.
I used bug number 2357 (P2) with a question mark as the reference, as I think after this fix 2357 can be closed, even though the fix does not directly address the reported problem.

- The right context menu is now shorter and has pinned/unpinned in it.
- Two menu items have moved to interface preferences (timeline)
- The pin-unpin button now makes a pop-up menu whether right or left clicked.
- The menu is now in the right place.
- The pop up menu now uses checkboxes rather than the text changing.

src/AdornedRulerPanel.cpp

@@ -372,11 +372,8 @@ void AdornedRulerPanel::QuickPlayIndicatorOverlay::Draw(
 enum {
    OnToggleQuickPlayID = 7000,
    OnSyncQuickPlaySelID,
-   OnTimelineToolTipID,
    OnAutoScrollID,
    OnLockPlayRegionID,
-   OnScrubRulerID,
-
    OnTogglePinnedStateID,
 };
 
@@ -388,14 +385,13 @@ BEGIN_EVENT_TABLE(AdornedRulerPanel, CellularPanel)
    // Context menu commands
    EVT_MENU(OnToggleQuickPlayID, AdornedRulerPanel::OnToggleQuickPlay)
    EVT_MENU(OnSyncQuickPlaySelID, AdornedRulerPanel::OnSyncSelToQuickPlay)
-   EVT_MENU(OnTimelineToolTipID, AdornedRulerPanel::OnTimelineToolTips)
    EVT_MENU(OnAutoScrollID, AdornedRulerPanel::OnAutoScroll)
    EVT_MENU(OnLockPlayRegionID, AdornedRulerPanel::OnLockPlayRegion)
-   EVT_MENU(OnScrubRulerID, AdornedRulerPanel::OnToggleScrubRulerFromMenu)
+   EVT_MENU( OnTogglePinnedStateID, AdornedRulerPanel::OnTogglePinnedState )
 
    EVT_COMMAND( OnTogglePinnedStateID,
                wxEVT_COMMAND_BUTTON_CLICKED,
-               AdornedRulerPanel::OnTogglePinnedState )
+               AdornedRulerPanel::OnPinnedButton )
 
 END_EVENT_TABLE()
 
@@ -974,6 +970,8 @@ void AdornedRulerPanel::UpdatePrefs()
    // Update button texts for language change
    UpdateButtonStates();
 
+   mTimelineToolTip = !!gPrefs->Read(wxT("/QuickPlay/ToolTips"), 1L);
+
 #ifdef EXPERIMENTAL_SCROLLING_LIMITS
 #ifdef EXPERIMENTAL_TWO_TONE_TIME_RULER
    {
@@ -1718,6 +1716,7 @@ void AdornedRulerPanel::StartQPPlay(bool looped, bool cutPreview)
    }
 }
 
+#if 0
 // This version toggles ruler state indirectly via the scrubber
 // to ensure that all the places where the state is shown update.
 // For example buttons and menus must update.
@@ -1726,6 +1725,8 @@ void AdornedRulerPanel::OnToggleScrubRulerFromMenu(wxCommandEvent&)
    auto &scrubber = Scrubber::Get( *mProject );
    scrubber.OnToggleScrubRuler(*mProject);
 }
+#endif
+
 
 bool AdornedRulerPanel::SetPanelSize()
 {
@@ -1784,6 +1785,11 @@ void AdornedRulerPanel::UpdateButtonStates()
    }
 }
 
+void AdornedRulerPanel::OnPinnedButton(wxCommandEvent & /*event*/)
+{
+   ShowContextMenu(MenuChoice::QuickPlay, NULL);
+}
+
 void AdornedRulerPanel::OnTogglePinnedState(wxCommandEvent & /*event*/)
 {
    TogglePinnedHead();
@@ -1817,42 +1823,24 @@ void AdornedRulerPanel::ShowMenu(const wxPoint & pos)
    const auto &playRegion = viewInfo.playRegion;
    wxMenu rulerMenu;
 
-   if (mQuickPlayEnabled)
-      rulerMenu.Append(OnToggleQuickPlayID, _("Disable Quick-Play"));
-   else
-      rulerMenu.Append(OnToggleQuickPlayID, _("Enable Quick-Play"));
+   rulerMenu.AppendCheckItem(OnToggleQuickPlayID, _("Enable Quick-Play"))->
+      Check(mQuickPlayEnabled);
 
-   wxMenuItem *dragitem;
-   if (mPlayRegionDragsSelection && !playRegion.Locked())
-      dragitem = rulerMenu.Append(OnSyncQuickPlaySelID, _("Disable dragging selection"));
-   else
-      dragitem = rulerMenu.Append(OnSyncQuickPlaySelID, _("Enable dragging selection"));
-   dragitem->Enable(mQuickPlayEnabled && !playRegion.Locked());
+   auto pDrag = rulerMenu.AppendCheckItem(OnSyncQuickPlaySelID, _("Enable dragging selection"));
+   pDrag->Check(mPlayRegionDragsSelection && !playRegion.Locked());
+   pDrag->Enable(mQuickPlayEnabled && !playRegion.Locked());
 
-#if wxUSE_TOOLTIPS
-   if (mTimelineToolTip)
-      rulerMenu.Append(OnTimelineToolTipID, _("Disable Timeline Tooltips"));
-   else
-      rulerMenu.Append(OnTimelineToolTipID, _("Enable Timeline Tooltips"));
-#endif
+   rulerMenu.AppendCheckItem(OnAutoScrollID, _("Update display while playing"))->
+      Check(mViewInfo->bUpdateTrackIndicator);
 
-   if (mViewInfo->bUpdateTrackIndicator)
-      rulerMenu.Append(OnAutoScrollID, _("Do not scroll while playing"));
-   else
-      rulerMenu.Append(OnAutoScrollID, _("Update display while playing"));
+   auto pLock = rulerMenu.AppendCheckItem(OnLockPlayRegionID, _("Lock Play Region"));
+   pLock->Check(playRegion.Locked());
+   pLock->Enable( playRegion.Locked() || !playRegion.Empty() );
 
-   wxMenuItem *prlitem;
-   if (!playRegion.Locked())
-      prlitem = rulerMenu.Append(OnLockPlayRegionID, _("Lock Play Region"));
-   else
-      prlitem = rulerMenu.Append(OnLockPlayRegionID, _("Unlock Play Region"));
-   prlitem->Enable( playRegion.Locked() || !playRegion.Empty() );
 
-   wxMenuItem *ruleritem;
-   if (ShowingScrubRuler())
-      ruleritem = rulerMenu.Append(OnScrubRulerID, _("Disable Scrub Ruler"));
-   else
-      ruleritem = rulerMenu.Append(OnScrubRulerID, _("Enable Scrub Ruler"));
+   rulerMenu.AppendSeparator();
+   rulerMenu.AppendCheckItem(OnTogglePinnedStateID, _("Pinned Play Head"))->
+      Check(TracksPrefs::GetPinnedHeadPreference());
 
    PopupMenu(&rulerMenu, pos);
 }
@@ -1905,12 +1893,14 @@ void AdornedRulerPanel::HandleSnapping()
    }
 }
 
+#if 0
 void AdornedRulerPanel::OnTimelineToolTips(wxCommandEvent&)
 {
    mTimelineToolTip = (mTimelineToolTip)? false : true;
    gPrefs->Write(wxT("/QuickPlay/ToolTips"), mTimelineToolTip);
    gPrefs->Flush();
 }
+#endif
 
 void AdornedRulerPanel::OnAutoScroll(wxCommandEvent&)
 {
@@ -2004,12 +1994,21 @@ void AdornedRulerPanel::ShowContextMenu( MenuChoice choice, const wxPoint *pPosi
    else
    {
       auto rect = GetRect();
-      position = { rect.GetLeft() + 1, rect.GetBottom() + 1 };
+      //Old code put menu too low down.  y position applied twice.
+      //position = { rect.GetLeft() + 1, rect.GetBottom() + 1 };
+
+      // The cell does not pass in the mouse or button position.
+      // We happen to know this is the pin/unpin button
+      // so these magic values 'fix a bug' - but really the cell should
+      // pass more information to work with in.
+      position = { rect.GetLeft() + 38, rect.GetHeight()/2 + 1 };
    }
 
    switch (choice) {
       case MenuChoice::QuickPlay:
-         ShowMenu(position); break;
+         ShowMenu(position); 
+         UpdateButtonStates();
+         break;
       case MenuChoice::Scrub:
          ShowScrubMenu(position); break;
       default:

src/AdornedRulerPanel.h

@@ -73,7 +73,7 @@ public:
    void UpdateQuickPlayPos(wxCoord &mousePosX, bool shiftDown);
 
    bool ShowingScrubRuler() const;
-   void OnToggleScrubRulerFromMenu(wxCommandEvent& );
+   //void OnToggleScrubRulerFromMenu(wxCommandEvent& );
    bool SetPanelSize();
    
    void DrawBothOverlays();
@@ -156,10 +156,11 @@ private:
    void HandleSnapping();
    void OnToggleQuickPlay(wxCommandEvent &evt);
    void OnSyncSelToQuickPlay(wxCommandEvent &evt);
-   void OnTimelineToolTips(wxCommandEvent &evt);
+   //void OnTimelineToolTips(wxCommandEvent &evt);
    void OnAutoScroll(wxCommandEvent &evt);
    void OnLockPlayRegion(wxCommandEvent &evt);
 
+   void OnPinnedButton(wxCommandEvent & event);
    void OnTogglePinnedState(wxCommandEvent & event);
 
    bool mPlayRegionDragsSelection;

src/prefs/GUIPrefs.cpp

@@ -249,6 +249,17 @@ void GUIPrefs::PopulateOrExchange(ShuttleGui & S)
    }
    S.EndStatic();
 
+   S.StartStatic(XO("Timeline"));
+   {
+      S.TieCheckBox(XO("Show Timeline Tooltips"),
+                    {wxT("/QuickPlay/ToolTips"),
+                     true});
+      S.TieCheckBox(XO("Show Scrub Ruler"),
+                    {wxT("/QuickPlay/ScrubbingEnabled"),
+                     true});
+   }
+   S.EndStatic();
+
    S.EndScroller();
 }