Bug Hunting Session
Bug 84277 - TOOLBAR: Invisible separator
Summary: TOOLBAR: Invisible separator
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: graphics stack (show other bugs)
Version:
(earliest affected)
Inherited From OOo
Hardware: Other All
: medium enhancement
Assignee: Maxim Monastirsky
URL:
Whiteboard: target:5.1.0
Keywords:
Depends on:
Blocks: Writer-Toolbars
  Show dependency treegraph
 
Reported: 2014-09-24 10:45 UTC by Yousuf Philips (jay) (retired)
Modified: 2017-06-27 14:44 UTC (History)
3 users (show)

See Also:
Crash report or crash signature:


Attachments
with and without the separtor (11.90 KB, image/png)
2014-09-24 10:45 UTC, Yousuf Philips (jay) (retired)
Details
screenshot (7.15 KB, image/png)
2015-09-02 02:43 UTC, Yousuf Philips (jay) (retired)
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Yousuf Philips (jay) (retired) 2014-09-24 10:45:15 UTC
Created attachment 106783 [details]
with and without the separtor

Visible separators are being used in the toolbars next to drop down menu and value controls when space is needed between it and the next button or control. This is evident in the formatting toolbar with the separators between font style, font name, font size. What is really needed is an invisible separator to provide the necessary spacing between these toolbar elements, as the separator is a means of separating elements that arent closely related to each other.
Comment 1 Yousuf Philips (jay) (retired) 2014-09-24 10:50:16 UTC
It would also be useful to be able to change the default number of pixels used by the separator in the form of an xml value in two possible manners.

1) total pixels - toolbar:size="10px"
2) pixels on either side - toolbar:size="5px,5px"
3) multiple of the default size - toolbar:size="*2"
Comment 2 Yousuf Philips (jay) (retired) 2014-09-26 04:52:14 UTC
Alternatively, we could have left padding values (of course when the UI is LTR and it would also need to work with both small and large icons) to the toolbar elements.
Comment 3 Adolfo Jayme 2014-12-03 03:14:19 UTC
It’s overkill to have “invisible separators”. Just remove them and have a separate change fixing the spacing between the drop-down boxes.
Comment 4 Adolfo Jayme 2015-08-13 14:16:04 UTC
I stand by my opinion on comment 3. Just fix the spacing without introducing useless workarounds.
Comment 5 Yousuf Philips (jay) (retired) 2015-08-14 15:56:20 UTC
(In reply to Adolfo Jayme from comment #4)
> I stand by my opinion on comment 3. Just fix the spacing without introducing
> useless workarounds.

Lets see what ux-advise thinks and it would be good to hear what devs think, as i think it maybe not as easy to fix as your suggestion, and it is not only limited to drop down menus - e.g. line width spin button in drawing toolbar.
Comment 6 Maxim Monastirsky 2015-08-16 17:50:10 UTC
It seems that we already have toolbar:toolbarspace, that can be used instead of toolbar:toolbarseparator. But it has some bugs, that should be fixed before we can make any use of it.

Another thing I noticed is that under Windows (as well as under Linux when running with SAL_USE_VCLPLUGIN=gen), the separators between the comboboxes aren't visible, although they are visible in other places on the toolbar. Would be useful to check whether it's just a bug, or it was done intentionally by someone, and in this case how could we reuse that for the GTK interface.

(In reply to Yousuf (Jay) Philips from comment #1)
> It would also be useful to be able to change the default number of pixels
> used by the separator in the form of an xml value in two possible manners.
> 
> 1) total pixels - toolbar:size="10px"
> 2) pixels on either side - toolbar:size="5px,5px"
> 3) multiple of the default size - toolbar:size="*2"
Seems overkill to me. We don't want to add such complexity just for 1 or 2 places where we want spaces.
Comment 7 Commit Notification 2015-08-22 22:09:01 UTC
Maxim Monastirsky committed a patch related to this issue.
It has been pushed to "master":

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

tdf#84277 Use the same logic for native separators

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 8 Yousuf Philips (jay) (retired) 2015-09-02 02:43:02 UTC
Created attachment 118322 [details]
screenshot

The patch seems to have gone wrong abit as entries set to <toolbar:toolbarseparator/> now no longer appear when they should be appearing.
Comment 9 Maxim Monastirsky 2015-09-02 07:26:59 UTC
(In reply to Yousuf (Jay) Philips from comment #8)
> The patch seems to have gone wrong abit as entries set to
> <toolbar:toolbarseparator/> now no longer appear when they should be
> appearing.
That was actually the behavior under Windows all that time, I just enabled it for GTK too. The separator becomes invisible if the previous item *or* the next item isn't a simple toolbar button (but a combobox or a spinbox or whatever). So the desired behavior is to make it invisible only if it has a control on both sided?

Also - is it something we need in the default toolbar layout? Because if not - I think it might be reasonable to expect from the user to use a workaround like putting there 2 separators. (The reason I'm asking is that placing a simple button near a control doesn't look good in some themes, especially when you hover the button, so we might want an invisible separator in such cases. But then again - if we don't use such configuration in the default layout, we probably shouldn't care about it, and make the separator visible like you're proposing.)
Comment 10 Yousuf Philips (jay) (retired) 2015-09-04 08:54:46 UTC
(In reply to Maxim Monastirsky from comment #9)
> That was actually the behavior under Windows all that time, I just enabled
> it for GTK too. The separator becomes invisible if the previous item *or*
> the next item isn't a simple toolbar button (but a combobox or a spinbox or
> whatever). So the desired behavior is to make it invisible only if it has a
> control on both sided?

Yes the behaviour should be that only when the previous and next controls are both comboboxes, spinboxes, etc. The drawing object properties toolbar in writer shows quite a bad behaviour as it is now.

> Also - is it something we need in the default toolbar layout? Because if not
> - I think it might be reasonable to expect from the user to use a workaround
> like putting there 2 separators. (The reason I'm asking is that placing a
> simple button near a control doesn't look good in some themes, especially
> when you hover the button, so we might want an invisible separator in such
> cases. But then again - if we don't use such configuration in the default
> layout, we probably shouldn't care about it, and make the separator visible
> like you're proposing.)

Ideally i dont think we should be automatically hiding separators when we have <toolbar:toolbarspace>, which we can use in places that we feel it should be there by default, as that is the default configuration all users get.
Comment 11 Maxim Monastirsky 2015-09-04 09:46:45 UTC
(In reply to Yousuf (Jay) Philips from comment #10)
> Yes the behaviour should be that only when the previous and next controls
> are both comboboxes, spinboxes, etc.
OK, so this change can be made as a first step.

> Ideally i dont think we should be automatically hiding separators when we
> have <toolbar:toolbarspace>
The main issue with toolbarspace right now is that it doesn't survive customization (it turns to a separator, and is shown in the customization dialog as such). Not sure how should we handle it in this context, if at all?
Comment 12 Commit Notification 2015-09-06 19:27:04 UTC
Maxim Monastirsky committed a patch related to this issue.
It has been pushed to "master":

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

Related: tdf#84277 Hide separator only between two windows

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.