Bug 91944 - Increase/Decrease indent sidebar buttons aren't working in Impress
Summary: Increase/Decrease indent sidebar buttons aren't working in Impress
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: UI (show other bugs)
Version:
(earliest affected)
5.1.0.0.alpha0+ Master
Hardware: Other All
: medium normal
Assignee: Maxim Monastirsky
QA Contact:
URL:
Whiteboard: target:5.1.0 target:5.0.0.1
Keywords:
Depends on:
Blocks:
 
Reported: 2015-06-08 18:54 UTC by Maxim Monastirsky
Modified: 2016-10-25 19:23 UTC (History)
2 users (show)

See Also:
Crash report or crash signature:


Attachments
quick patch (for demonstration, do not push) (28.41 KB, patch)
2015-06-08 21:28 UTC, Maxim Monastirsky
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Maxim Monastirsky 2015-06-08 18:54:18 UTC
Since the following commit:

commit ed6b7d972bf1aee323947f22c6b5de430db4a9a5
Author: Yousuf Philips <philipz85@hotmail.com>
Date:   Tue Jun 2 13:40:45 2015 +0400

    Fix indent button icons in impress/draw sidebar
    
    Change-Id: Ib21c2489e1ff420e651e50a2731dfd3f16c9cef8
    Reviewed-on: https://gerrit.libreoffice.org/16028
    Reviewed-by: Caolán McNamara <caolanm@redhat.com>
    Tested-by: Caolán McNamara <caolanm@redhat.com>

These buttons do nothing when clicking them. Note that before that change, these buttons actually did Promote/Demote much like .uno:OutlineLeft\.uno:OutlineRight, just the tooltips were wrong.

So we should either revert this change, or if we really want to change the behavior of these buttons to Increase/Decrease indent, we should somehow make them work.

Jay: Any thoughts on this?
Comment 1 Yousuf Philips (jay) 2015-06-08 20:18:46 UTC
Hi Maxim,

Yes this was something that i noticed and wanted to look into. The old buttons were for promote and demote outline though they are placed under the indent section, while increase and decrease sidebar buttons work in writer and calc. These buttons should work as you can change the indent value in the spin buttons in the sidebar or in the paragraph dialog. So what are your thoughts?
Comment 2 Maxim Monastirsky 2015-06-08 21:28:32 UTC
Created attachment 116389 [details]
quick patch (for demonstration, do not push)

Hi Jay,

(In reply to Yousuf (Jay) Philips from comment #1)
> while increase and decrease sidebar buttons work in writer
> and calc. These buttons should work as you can change the indent value in
> the spin buttons in the sidebar or in the paragraph dialog.
Sounds like you prefer the second option (i.e. change the behavior to inc/dec indent). In this case, the sidebar code needs to be patched with something like the attached patch. You can test it if you have your own build, but please do not push, as this needs cleanup and verifying that it doesn't break anything else. If that's the route you want to take, just let me know, and I'll try to find some time to finish it in the coming days.
Comment 3 Maxim Monastirsky 2015-06-08 21:29:36 UTC
And set to NEW, to not confuse the QA
Comment 4 Yousuf Philips (jay) 2015-06-09 11:37:51 UTC
(In reply to Maxim Monastirsky from comment #2)
> Sounds like you prefer the second option (i.e. change the behavior to
> inc/dec indent).

Yes for consistency between apps, the buttons should be inc/dec indent buttons and should be functional. We should also add promote, demote, move up, move down buttons into the paragraph section (bug 87651).

> In this case, the sidebar code needs to be patched with
> something like the attached patch. You can test it if you have your own
> build, but please do not push, as this needs cleanup and verifying that it
> doesn't break anything else. If that's the route you want to take, just let
> me know, and I'll try to find some time to finish it in the coming days.

I do have my own build but dont know how to apply your patch into the build. When a patch is in gerrit that i want to test, i'm easily able to download the patch into my build with './logerrit cherry-pick'.
Comment 5 Maxim Monastirsky 2015-06-09 13:48:56 UTC
(In reply to Yousuf (Jay) Philips from comment #4)
> Yes for consistency between apps, the buttons should be inc/dec indent
> buttons and should be functional.
Fine. I'll look into it tomorrow.

> I do have my own build but dont know how to apply your patch into the build.
$ git am patch_file_name
Comment 6 Commit Notification 2015-06-10 08:30:39 UTC
Maxim Monastirsky committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=a861a234b0f5f39b06fd6180655182c167c3a94d

tdf#91944 Fix inc/dec indent buttons in impress

It will be available in 5.1.0.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds
Affected users are encouraged to test the fix and report feedback.
Comment 7 Yousuf Philips (jay) 2015-06-10 09:39:37 UTC
Shouldnt we leave the code in for the promote and demote, so that it can be added next to the bullet and numbering buttons?
Comment 8 Maxim Monastirsky 2015-06-10 09:52:43 UTC
(In reply to Yousuf (Jay) Philips from comment #7)
> Shouldnt we leave the code in for the promote and demote, so that it can be
> added next to the bullet and numbering buttons?
No. There is much simpler way to do it.
Comment 9 Commit Notification 2015-06-16 11:42:08 UTC
Maxim Monastirsky committed a patch related to this issue.
It has been pushed to "libreoffice-5-0":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=07c845ffc43c4472c36b2912954f3730e2ed6c0b&h=libreoffice-5-0

tdf#91944 Fix inc/dec indent buttons in impress

It will be available in 5.0.0.1.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds
Affected users are encouraged to test the fix and report feedback.