tdf#133569 rework the SwCommentRuler button

The original design was planned in tdf#38246 and documented as:
https://wiki.documentfoundation.org/Design/Whiteboards/Comments_Ruler_Control

This patch includes the following changes:
* use the correct highlight colors
* use a down-pointing arrow for expanse with a stable position,
like some tree view implementations
* left-align the label in RTL for better us- / read-ability
* dynamic sized arrow for HiDPI:
* draw arrow using a polygon
* calculate arrow size and padding based on font size
* drop almost all constant values
* fix the instable, mouse-over tooltip -> just show it always

While at it, I found clang-format just added some minimal,
additional changes, so I removed the file from the blacklist
and included these.

Change-Id: I6e0bade387639eae6bdaaf7960b2fe44c73adf65
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/75421
Tested-by: Jenkins
Reviewed-by: Heiko Tietze <heiko.tietze@documentfoundation.org>
Reviewed-by: Jan-Marek Glogowski <glogow@fbihome.de>

1 files changed, 132 insertions, 146 deletions

diff --git a/sw/source/uibase/misc/swruler.cxx b/sw/source/uibase/misc/swruler.cxx
index 1cf12719c3d3..680031a264b8 100644
--- a/sw/source/uibase/misc/swruler.cxx
+++ b/sw/source/uibase/misc/swruler.cxx
@@ -7,9 +7,7 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/.
  */
 
-// FIX fdo#38246 https://bugs.libreoffice.org/show_bug.cgi?id=38246
 // Design proposal: https://wiki.documentfoundation.org/Design/Whiteboards/Comments_Ruler_Control
-// TODO Alpha blend border when it doesn't fit in window
 
 #include <swruler.hxx>
 
@@ -28,66 +26,79 @@
 #include <LibreOfficeKit/LibreOfficeKitEnums.h>
 #include <boost/property_tree/json_parser.hpp>
 
-#define CONTROL_BORDER_WIDTH    1
-#define CONTROL_LEFT_OFFSET     6
-#define CONTROL_RIGHT_OFFSET    3
-#define CONTROL_TOP_OFFSET      4
-
-#define CONTROL_TRIANGLE_WIDTH  4
-#define CONTROL_TRIANGLE_PAD    3
-
-namespace {
+#define CONTROL_BORDER_WIDTH 1
 
+namespace
+{
 /**
- * Draw a little horizontal arrow tip on VirtualDevice.
- * \param nX left coordinate of arrow
- * \param nY top coordinate of arrow
+ * Draw a little arrow / triangle with different directions
+ *
+ * \param nX left coordinate of arrow square
+ * \param nY top coordinate of arrow square
+ * \param nSize size of the long triangle side / arrow square
  * \param Color arrow color
- * \param bPointRight if arrow should point to right. Otherwise, it will point left.
+ * \param bCollapsed if the arrow should display the collapsed state
  */
-void ImplDrawArrow(vcl::RenderContext& rRenderContext, long nX, long nY, const Color& rColor, bool bPointRight)
+void ImplDrawArrow(vcl::RenderContext& rRenderContext, long nX, long nY, long nSize,
+                   const Color& rColor, bool bCollapsed)
 {
-    rRenderContext.SetLineColor();
-    rRenderContext.SetFillColor(rColor);
-    if (bPointRight)
+    tools::Polygon aTriaglePolygon(4);
+
+    if (bCollapsed)
     {
-        rRenderContext.DrawRect(tools::Rectangle(nX + 0, nY + 0, nX + 0, nY + 6) );
-        rRenderContext.DrawRect(tools::Rectangle(nX + 1, nY + 1, nX + 1, nY + 5) );
-        rRenderContext.DrawRect(tools::Rectangle(nX + 2, nY + 2, nX + 2, nY + 4) );
-        rRenderContext.DrawRect(tools::Rectangle(nX + 3, nY + 3, nX + 3, nY + 3) );
+        if (AllSettings::GetLayoutRTL()) // <
+        {
+            aTriaglePolygon.SetPoint({ nX + nSize / 2, nY }, 0);
+            aTriaglePolygon.SetPoint({ nX + nSize / 2, nY + nSize }, 1);
+            aTriaglePolygon.SetPoint({ nX, nY + nSize / 2 }, 2);
+            aTriaglePolygon.SetPoint({ nX + nSize / 2, nY }, 3);
+        }
+        else // >
+        {
+            aTriaglePolygon.SetPoint({ nX, nY }, 0);
+            aTriaglePolygon.SetPoint({ nX + nSize / 2, nY + nSize / 2 }, 1);
+            aTriaglePolygon.SetPoint({ nX, nY + nSize }, 2);
+            aTriaglePolygon.SetPoint({ nX, nY }, 3);
+        }
     }
-    else
+    else // v
     {
-        rRenderContext.DrawRect(tools::Rectangle(nX + 0, nY + 3, nX + 0, nY + 3));
-        rRenderContext.DrawRect(tools::Rectangle(nX + 1, nY + 2, nX + 1, nY + 4));
-        rRenderContext.DrawRect(tools::Rectangle(nX + 2, nY + 1, nX + 2, nY + 5));
-        rRenderContext.DrawRect(tools::Rectangle(nX + 3, nY + 0, nX + 3, nY + 6));
+        aTriaglePolygon.SetPoint({ nX, nY + nSize / 2 }, 0);
+        aTriaglePolygon.SetPoint({ nX + nSize, nY + nSize / 2 }, 1);
+        aTriaglePolygon.SetPoint({ nX + nSize / 2, nY + nSize }, 2);
+        aTriaglePolygon.SetPoint({ nX, nY + nSize / 2 }, 3);
     }
-}
 
+    rRenderContext.SetLineColor();
+    rRenderContext.SetFillColor(rColor);
+    rRenderContext.DrawPolygon(aTriaglePolygon);
+}
 }
 
 // Constructor
-SwCommentRuler::SwCommentRuler( SwViewShell* pViewSh, vcl::Window* pParent, SwEditWin* pWin, SvxRulerSupportFlags nRulerFlags,  SfxBindings& rBindings, WinBits nWinStyle)
-: SvxRuler(pParent, pWin, nRulerFlags, rBindings, nWinStyle | WB_HSCROLL)
-, mpViewShell(pViewSh)
-, mpSwWin(pWin)
-, mbIsHighlighted(false)
-, mnFadeRate(0)
-, maVirDev( VclPtr<VirtualDevice>::Create(*this) )
+SwCommentRuler::SwCommentRuler(SwViewShell* pViewSh, vcl::Window* pParent, SwEditWin* pWin,
+                               SvxRulerSupportFlags nRulerFlags, SfxBindings& rBindings,
+                               WinBits nWinStyle)
+    : SvxRuler(pParent, pWin, nRulerFlags, rBindings, nWinStyle | WB_HSCROLL)
+    , mpViewShell(pViewSh)
+    , mpSwWin(pWin)
+    , mbIsHighlighted(false)
+    , mnFadeRate(0)
+    , maVirDev(VclPtr<VirtualDevice>::Create(*this))
 {
     // Set fading timeout: 5 x 40ms = 200ms
     maFadeTimer.SetTimeout(40);
-    maFadeTimer.SetInvokeHandler( LINK( this, SwCommentRuler, FadeHandler ) );
-    maFadeTimer.SetDebugName( "sw::SwCommentRuler maFadeTimer" );
-}
+    maFadeTimer.SetInvokeHandler(LINK(this, SwCommentRuler, FadeHandler));
+    maFadeTimer.SetDebugName("sw::SwCommentRuler maFadeTimer");
 
-// Destructor
-SwCommentRuler::~SwCommentRuler()
-{
-    disposeOnce();
+    // we have a little bit more space, as we don't draw ruler ticks
+    vcl::Font aFont(maVirDev->GetFont());
+    aFont.SetFontHeight(aFont.GetFontHeight() + 1);
+    maVirDev->SetFont(aFont);
 }
 
+SwCommentRuler::~SwCommentRuler() { disposeOnce(); }
+
 void SwCommentRuler::dispose()
 {
     mpSwWin.clear();
@@ -109,140 +120,111 @@ void SwCommentRuler::Paint(vcl::RenderContext& rRenderContext, const tools::Rect
 void SwCommentRuler::DrawCommentControl(vcl::RenderContext& rRenderContext)
 {
     const StyleSettings& rStyleSettings = rRenderContext.GetSettings().GetStyleSettings();
-    bool bIsCollapsed = ! mpViewShell->GetPostItMgr()->ShowNotes();
+    const bool bIsCollapsed = !mpViewShell->GetPostItMgr()->ShowNotes();
+    const tools::Rectangle aControlRect = GetCommentControlRegion();
 
-    tools::Rectangle aControlRect = GetCommentControlRegion();
     maVirDev->SetOutputSizePixel(aControlRect.GetSize());
 
-    // Paint comment control background
-    // TODO Check if these are best colors to be used
-    Color aBgColor = GetFadedColor( rStyleSettings.GetDialogColor(), rStyleSettings.GetWorkspaceColor() );
-    maVirDev->SetFillColor( aBgColor );
-
-    if ( mbIsHighlighted || !bIsCollapsed )
+    // set colors
+    if (!bIsCollapsed)
     {
-        // Draw borders
-        maVirDev->SetLineColor( rStyleSettings.GetShadowColor() );
+        if (mbIsHighlighted)
+            maVirDev->SetFillColor(
+                GetFadedColor(rStyleSettings.GetHighlightColor(), rStyleSettings.GetDialogColor()));
+        else
+            maVirDev->SetFillColor(rStyleSettings.GetDialogColor());
+        maVirDev->SetLineColor(rStyleSettings.GetShadowColor());
     }
     else
     {
-        // No borders
+        if (mbIsHighlighted)
+            maVirDev->SetFillColor(GetFadedColor(rStyleSettings.GetHighlightColor(),
+                                                 rStyleSettings.GetWorkspaceColor()));
+        else
+            maVirDev->SetFillColor(rStyleSettings.GetWorkspaceColor());
         maVirDev->SetLineColor();
     }
+    Color aTextColor = GetFadedColor(rStyleSettings.GetHighlightTextColor(),
+                                     rStyleSettings.GetButtonTextColor());
+    maVirDev->SetTextColor(aTextColor);
 
-    maVirDev->DrawRect( tools::Rectangle( Point(), aControlRect.GetSize() ) );
-
-    // Label and arrow tip
-    OUString aLabel( SwResId ( STR_COMMENTS_LABEL ) );
-    // Get label and arrow coordinates
-    Point aLabelPos;
-    Point aArrowPos;
-    bool  bArrowToRight;
-    // TODO Discover why it should be 0 instead of CONTROL_BORDER_WIDTH + CONTROL_TOP_OFFSET
-    aLabelPos.setY( 0 );
-    aArrowPos.setY( CONTROL_BORDER_WIDTH + CONTROL_TOP_OFFSET );
-    if ( !AllSettings::GetLayoutRTL() )
+    // calculate label and arrow positions
+    const OUString aLabel = SwResId(STR_COMMENTS_LABEL);
+    const long nTriangleSize = maVirDev->GetTextHeight() / 2 + 1;
+    const long nTrianglePad = maVirDev->GetTextHeight() / 4;
+
+    Point aLabelPos(0, (aControlRect.GetHeight() - maVirDev->GetTextHeight()) / 2);
+    Point aArrowPos(0, (aControlRect.GetHeight() - nTriangleSize) / 2);
+
+    if (!AllSettings::GetLayoutRTL()) // | > Comments |
     {
-        // LTR
-        if ( bIsCollapsed )
-        {
-            // It should draw something like | > Comments  |
-            aLabelPos.setX( CONTROL_LEFT_OFFSET + CONTROL_TRIANGLE_WIDTH + CONTROL_TRIANGLE_PAD );
-            aArrowPos.setX( CONTROL_LEFT_OFFSET );
-        }
-        else
-        {
-            // It should draw something like | Comments  < |
-            aLabelPos.setX( CONTROL_LEFT_OFFSET );
-            aArrowPos.setX( aControlRect.GetSize().Width() - 1 - CONTROL_RIGHT_OFFSET - CONTROL_BORDER_WIDTH - CONTROL_TRIANGLE_WIDTH );
-        }
-        bArrowToRight = bIsCollapsed;
+        aArrowPos.setX(nTrianglePad);
+        aLabelPos.setX(aArrowPos.X() + nTriangleSize + nTrianglePad);
     }
-    else
+    else // RTL => | Comments < |
     {
-        // RTL
-        long nLabelWidth = GetTextWidth( aLabel );
-        if ( bIsCollapsed )
+        const long nLabelWidth = maVirDev->GetTextWidth(aLabel);
+        if (!bIsCollapsed)
         {
-            // It should draw something like |  Comments < |
-            aArrowPos.setX( aControlRect.GetSize().Width() - 1 - CONTROL_RIGHT_OFFSET - CONTROL_BORDER_WIDTH - CONTROL_TRIANGLE_WIDTH );
-            aLabelPos.setX( aArrowPos.X() - CONTROL_TRIANGLE_PAD - nLabelWidth );
+            aArrowPos.setX(aControlRect.GetWidth() - 1 - nTrianglePad - CONTROL_BORDER_WIDTH
+                           - nTriangleSize);
+            aLabelPos.setX(aArrowPos.X() - nTrianglePad - nLabelWidth);
         }
         else
         {
-            // It should draw something like | >  Comments |
-            aLabelPos.setX( aControlRect.GetSize().Width() - 1 - CONTROL_RIGHT_OFFSET - CONTROL_BORDER_WIDTH - nLabelWidth );
-            aArrowPos.setX( CONTROL_LEFT_OFFSET );
+            // if comments are collapsed, left align the text, because otherwise it's very likely to be invisible
+            aArrowPos.setX(nLabelWidth + nTrianglePad + nTriangleSize);
+            aLabelPos.setX(aArrowPos.X() - nTrianglePad - nLabelWidth);
         }
-        bArrowToRight = !bIsCollapsed;
     }
 
-    // Draw label
-    Color aTextColor = GetFadedColor( rStyleSettings.GetButtonTextColor(), rStyleSettings.GetDarkShadowColor() );
-    maVirDev->SetTextColor( aTextColor );
-    // FIXME Expected font size?
-    maVirDev->DrawText( aLabelPos, aLabel );
-
-    // Draw arrow
-    // FIXME consistence of button colors. https://opengrok.libreoffice.org/xref/core/vcl/source/control/button.cxx#785
-    ImplDrawArrow(*maVirDev, aArrowPos.X(), aArrowPos.Y(), aTextColor, bArrowToRight);
-
-    // Blit comment control
-    rRenderContext.DrawOutDev(aControlRect.TopLeft(), aControlRect.GetSize(), Point(), aControlRect.GetSize(), *maVirDev);
+    // draw control
+    maVirDev->DrawRect(tools::Rectangle(Point(), aControlRect.GetSize()));
+    maVirDev->DrawText(aLabelPos, aLabel);
+    ImplDrawArrow(*maVirDev, aArrowPos.X(), aArrowPos.Y(), nTriangleSize, aTextColor, bIsCollapsed);
+    rRenderContext.DrawOutDev(aControlRect.TopLeft(), aControlRect.GetSize(), Point(),
+                              aControlRect.GetSize(), *maVirDev);
 }
 
 // Just accept double-click outside comment control
-void SwCommentRuler::Command( const CommandEvent& rCEvt )
+void SwCommentRuler::Command(const CommandEvent& rCEvt)
 {
     Point aMousePos = rCEvt.GetMousePosPixel();
     // Ignore command request if it is inside Comment Control
-    if ( !mpViewShell->GetPostItMgr()
-          || !mpViewShell->GetPostItMgr()->HasNotes()
-          || !GetCommentControlRegion().IsInside( aMousePos ) )
-        SvxRuler::Command( rCEvt );
+    if (!mpViewShell->GetPostItMgr() || !mpViewShell->GetPostItMgr()->HasNotes()
+        || !GetCommentControlRegion().IsInside(aMousePos))
+        SvxRuler::Command(rCEvt);
 }
 
 void SwCommentRuler::MouseMove(const MouseEvent& rMEvt)
 {
     SvxRuler::MouseMove(rMEvt);
-    if ( ! mpViewShell->GetPostItMgr() || ! mpViewShell->GetPostItMgr()->HasNotes() )
+    if (!mpViewShell->GetPostItMgr() || !mpViewShell->GetPostItMgr()->HasNotes())
         return;
 
+    UpdateCommentHelpText();
+
     Point aMousePos = rMEvt.GetPosPixel();
-    bool  bWasHighlighted = mbIsHighlighted;
-    mbIsHighlighted = GetCommentControlRegion().IsInside( aMousePos );
-    if ( mbIsHighlighted != bWasHighlighted )
-    {
-        // Set proper help text
-        if ( mbIsHighlighted )
-        {
-            // Mouse over comment control
-            UpdateCommentHelpText();
-        }
-        else
-        {
-            // Mouse out of comment control
-            // FIXME Should remember previous tooltip text?
-            SetQuickHelpText( OUString() );
-        }
+    bool bWasHighlighted = mbIsHighlighted;
+    mbIsHighlighted = GetCommentControlRegion().IsInside(aMousePos);
+    if (mbIsHighlighted != bWasHighlighted)
         // Do start fading
         maFadeTimer.Start();
-    }
 }
 
-void SwCommentRuler::MouseButtonDown( const MouseEvent& rMEvt )
+void SwCommentRuler::MouseButtonDown(const MouseEvent& rMEvt)
 {
-    Point   aMousePos = rMEvt.GetPosPixel();
-    if ( !rMEvt.IsLeft() || IsTracking() || !GetCommentControlRegion().IsInside( aMousePos ) )
+    Point aMousePos = rMEvt.GetPosPixel();
+    if (!rMEvt.IsLeft() || IsTracking() || !GetCommentControlRegion().IsInside(aMousePos))
     {
         SvxRuler::MouseButtonDown(rMEvt);
         return;
     }
 
     // Toggle notes visibility
-    SwView &rView = mpSwWin->GetView();
-    SfxRequest aRequest( rView.GetViewFrame(), SID_TOGGLE_NOTES );
-    rView.ExecViewOptions( aRequest );
+    SwView& rView = mpSwWin->GetView();
+    SfxRequest aRequest(rView.GetViewFrame(), SID_TOGGLE_NOTES);
+    rView.ExecViewOptions(aRequest);
 
     // It is inside comment control, so update help text
     UpdateCommentHelpText();
@@ -307,7 +289,8 @@ void SwCommentRuler::NotifyKit()
         return;
 
     const std::string test = CreateJsonNotification();
-    mpViewShell->GetSfxViewShell()->libreOfficeKitViewCallback(LOK_CALLBACK_RULER_UPDATE, test.c_str());
+    mpViewShell->GetSfxViewShell()->libreOfficeKitViewCallback(LOK_CALLBACK_RULER_UPDATE,
+                                                               test.c_str());
 }
 
 void SwCommentRuler::Update()
@@ -322,7 +305,7 @@ void SwCommentRuler::Update()
 void SwCommentRuler::UpdateCommentHelpText()
 {
     const char* pTooltipResId;
-    if ( mpViewShell->GetPostItMgr()->ShowNotes() )
+    if (mpViewShell->GetPostItMgr()->ShowNotes())
         pTooltipResId = STR_HIDE_COMMENTS;
     else
         pTooltipResId = STR_SHOW_COMMENTS;
@@ -332,8 +315,7 @@ void SwCommentRuler::UpdateCommentHelpText()
 // TODO Make Ruler return its central rectangle instead of margins.
 tools::Rectangle SwCommentRuler::GetCommentControlRegion()
 {
-    long nLeft = 0;
-    SwPostItMgr *pPostItMgr = mpViewShell->GetPostItMgr();
+    SwPostItMgr* pPostItMgr = mpViewShell->GetPostItMgr();
 
     //rhbz#1006850 When the SwPostItMgr ctor is called from SwView::SwView it
     //triggers an update of the uiview, but the result of the ctor hasn't been
@@ -341,22 +323,26 @@ tools::Rectangle SwCommentRuler::GetCommentControlRegion()
     if (!pPostItMgr)
         return tools::Rectangle();
 
-    unsigned long nSidebarWidth = pPostItMgr->GetSidebarWidth(true);
+    const unsigned long nSidebarWidth = pPostItMgr->GetSidebarWidth(true);
+
     //FIXME When the page width is larger then screen, the ruler is misplaced by one pixel
+    long nLeft = GetPageOffset();
     if (GetTextRTL())
-       nLeft = GetPageOffset() - nSidebarWidth + GetBorderOffset();
+        nLeft += GetBorderOffset() - nSidebarWidth;
     else
-       nLeft = GetWinOffset() + GetPageOffset() + mpSwWin->LogicToPixel(Size(GetPageWidth(), 0)).Width();
-    long nTop    = 0 + 4; // Ruler::ImplDraw uses RULER_OFF (value: 3px) as offset, and Ruler::ImplFormat adds one extra pixel
+        nLeft += GetWinOffset() + mpSwWin->LogicToPixel(Size(GetPageWidth(), 0)).Width();
+
+    // Ruler::ImplDraw uses RULER_OFF (value: 3px) as offset, and Ruler::ImplFormat adds one extra pixel
+    long nTop = 4;
     // Somehow pPostItMgr->GetSidebarBorderWidth() returns border width already doubled
-    long nRight  = nLeft + nSidebarWidth + pPostItMgr->GetSidebarBorderWidth(true);
+    long nRight = nLeft + nSidebarWidth + pPostItMgr->GetSidebarBorderWidth(true);
     long nBottom = nTop + GetRulerVirHeight() - 3;
 
     tools::Rectangle aRect(nLeft, nTop, nRight, nBottom);
     return aRect;
 }
 
-Color SwCommentRuler::GetFadedColor(const Color &rHighColor, const Color &rLowColor)
+Color SwCommentRuler::GetFadedColor(const Color& rHighColor, const Color& rLowColor)
 {
     if (!maFadeTimer.IsActive())
         return mbIsHighlighted ? rHighColor : rLowColor;
@@ -366,19 +352,19 @@ Color SwCommentRuler::GetFadedColor(const Color &rHighColor, const Color &rLowCo
     return aColor;
 }
 
-IMPL_LINK_NOARG(SwCommentRuler, FadeHandler, Timer *, void)
+IMPL_LINK_NOARG(SwCommentRuler, FadeHandler, Timer*, void)
 {
     const int nStep = 25;
-    if ( mbIsHighlighted && mnFadeRate < 100 )
+    if (mbIsHighlighted && mnFadeRate < 100)
         mnFadeRate += nStep;
-    else if ( !mbIsHighlighted && mnFadeRate > 0 )
+    else if (!mbIsHighlighted && mnFadeRate > 0)
         mnFadeRate -= nStep;
     else
         return;
 
     Invalidate();
 
-    if ( mnFadeRate != 0 && mnFadeRate != 100)
+    if (mnFadeRate != 0 && mnFadeRate != 100)
         maFadeTimer.Start();
 }