Bug Hunting Session
Bug 109100 - SIDEBAR: Margin controls in Slide/Page content panel
Summary: SIDEBAR: Margin controls in Slide/Page content panel
Status: RESOLVED WONTFIX
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Draw (show other bugs)
Version:
(earliest affected)
6.0.0.0.alpha0+
Hardware: All All
: medium normal
Assignee: Not Assigned
URL:
Whiteboard: target:6.0.0 target:6.1.0 target:6.0.0.2
Keywords:
Depends on:
Blocks: Sidebar-Properties-Slide
  Show dependency treegraph
 
Reported: 2017-07-13 12:15 UTC by Yousuf Philips (jay) (retired)
Modified: 2018-07-06 20:38 UTC (History)
7 users (show)

See Also:
Crash report or crash signature:


Attachments
New dropdown for margins in Impress (16.88 KB, image/png)
2017-09-30 08:09 UTC, Heiko Tietze
Details
margin label in impress (23.39 KB, image/png)
2017-10-10 11:26 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) 2017-07-13 12:15:25 UTC
So 4 margin spinboxes[1] were added to the slide/page content panel and though it was intended only for Draw based on the commit title, it has also appeared in Impress were it doesnt look good.

So lets use this bug report to decide what needs to happen in each module.

In Draw, i suggest that

1. margins appear above master page
2. the 4 controls be replaced by a single margins drop down list box with useful presets, similar to how we do it in the Page tab/deck in Writer

In Impress, i would suggest it appear in the same way as in Draw, though i dont see many users changing margins on a regular basis for presentations and we dont have sufficient space for its addition with the Layouts content panel below it for our minimum target resolution.

[1] https://gerrit.libreoffice.org/#/c/37626/
Comment 1 Commit Notification 2017-09-30 06:33:30 UTC
pv2k committed a patch related to this issue.
It has been pushed to "master":

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

tdf#109100 margin spinboxes in draw's sidebar changed to listbox

It will be available in 6.0.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 2 Heiko Tietze 2017-09-30 08:09:46 UTC
Created attachment 136636 [details]
New dropdown for margins in Impress

Thank, pv2k. Looks pretty nice.

What we should discuss now are the defaults.

None = 0/0/0/0 (L/R/T/B according the dialog)
Narrow = 0.25/0.25/0.25/0.25
Moderate = 0.38/0.38/0.5/0.5
Normal 0.75" = 0.39/0.39/0.39/0.39
Normal 1" = 0.5/0.5/0.5/0.5
Normal 1.25" = 0.63/0.63/0.5/0.5
Wide = 1.0/1.0/0.5/0.5

IMHO we should either go with numbers and do something like "Narrow 0.25", "Moderate 0.38", and "Wide 2.0" (hopefully the numbers do not interfere with l10n), or we use just speaking names, what I like. "None, Narrow, Moderate, Average, Normal, Large, Wide" without any number.

If we do numbers I wonder if "Normal 0.75" is better than "Normal 0.39". Took me a moment to understand that there was no mistake.

In Writer (not changed with this patch) it is
None = 0,0,0,0
Narrow = 0.5/0.5/0.5/0.5
Moderate = 0.75/0.75/1/1
Normal 0.75" = 0.79/0.79/0.79/0.79
Normal 1" = 1/1/1/1
Normal 1.25" = 1.25/1.25/1/1
Wide = 2.0/2.0/1/1
Mirrored = 1.25/1/1/1

Taking the results from the margins survey [1] seriously we can enter some decent numbers here that work in most cases.

Small Margins = 0.5/0.5/0.5/0.5 inch respectively 1/1/1/1 cm
LibreOffice Margins = 0.79/0.79/0.79/0.79 in = 2/2/2/2 cm
Common Margins = 1/1/1/1 in = 2.54/2.54/2.54/2.54 cm
Binding = 0.79/0.5/0.5/0.5 in = 2/1/1/1 cm
Notes = 0.5/0.79/0.5/0.5 in = 1/2/1/1 cm
US Book = 1.25/1/1.25/1 in = 3.175/2.54/3.175/2.54 cm
ISO Book = 0.79/0.5/0.79/0.5 in = 2/1/2/1 cm
(Without the page dimension there is no way to insert values according de Graaf; US/ISO Book could also be Large Book/Small Book)

(Most users will still mess this up with the spacing at paragraph level.)

[1] https://design.blog.documentfoundation.org/2017/09/26/default-margins-users-apply-libreoffice-writer/
Comment 3 Yousuf Philips (jay) (retired) 2017-09-30 13:19:28 UTC
Glad to see the patch got pushed Vidhey, but as mentioned in my last comment on gerrit, please hide this field in impress and have it only visible in draw.

(In reply to Heiko Tietze from comment #2)
> None = 0/0/0/0 (L/R/T/B according the dialog)
> Narrow = 0.25/0.25/0.25/0.25
> Moderate = 0.38/0.38/0.5/0.5
> Normal 0.75" = 0.39/0.39/0.39/0.39
> Normal 1" = 0.5/0.5/0.5/0.5
> Normal 1.25" = 0.63/0.63/0.5/0.5
> Wide = 1.0/1.0/0.5/0.5

This looks off to me. Vidhey was this the list i gave you as the labels dont look right at all, as 'Normal 0.75"' should be something like 'Normal 1cm'.

> Small Margins = 0.5/0.5/0.5/0.5 inch respectively 1/1/1/1 cm
> LibreOffice Margins = 0.79/0.79/0.79/0.79 in = 2/2/2/2 cm
> Common Margins = 1/1/1/1 in = 2.54/2.54/2.54/2.54 cm
> Binding = 0.79/0.5/0.5/0.5 in = 2/1/1/1 cm
> Notes = 0.5/0.79/0.5/0.5 in = 1/2/1/1 cm
> US Book = 1.25/1/1.25/1 in = 3.175/2.54/3.175/2.54 cm
> ISO Book = 0.79/0.5/0.79/0.5 in = 2/1/2/1 cm
> (Without the page dimension there is no way to insert values according de
> Graaf; US/ISO Book could also be Large Book/Small Book)

Margins for writer documents are quite different to margins for draw illustrations, so these are quite irrelevant here.
Comment 4 Yousuf Philips (jay) (retired) 2017-09-30 13:37:06 UTC
Okay i found the margins i gave you on telegram - None, 0.5cm, 1cm, 0.5" (1.27cm), 2cm.
Comment 5 V Stuart Foote 2017-09-30 15:16:57 UTC
Vidhey, Bubli, *

As implemented the listbox labeling of Normal 0.75" is really a 1cm margin, and the Normal 1" is really a 1/2" margin. 

+#define SDPAGE_NO_MARGIN       0
+#define SDPAGE_NARROW_VALUE    635  => 1/4"
+#define SDPAGE_MODERATE_LR     955  => ~3/8"
+#define SDPAGE_NORMAL_VALUE    1000 => 1 cm
+#define SDPAGE_WIDE_VALUE1     1270 => 1/2"
+#define SDPAGE_WIDE_VALUE2     2540 => 1"
+#define SDPAGE_WIDE_VALUE3     1590 => ~5/8"

With these definitions, the listbox gives just US Imperial measure for the draw canvas and impress slide margins except for the 1cm NORMAL075.

Seems we need to provide some mix of Metric margins as well. 

Suggest we include 500, 750, 1250, 1500, 2000 values for .5cm, .75cm, 1.25cm, 1.5cm and 2cm; and then add the additional metric listbox items.

Ordering and labeling in the .ui could then be cleanly composed with cm and " marks. No need then for l10n.
Comment 6 V Stuart Foote 2017-09-30 15:21:01 UTC
(In reply to Yousuf Philips (jay) from comment #3)
> Glad to see the patch got pushed Vidhey, but as mentioned in my last comment
> on gerrit, please hide this field in impress and have it only visible in
> draw.
> 

Yes not commonly needed in Impress, and default for both Draw and Impress documents really should be the 0 of None.

> Margins for writer documents are quite different to margins for draw
> illustrations, so these are quite irrelevant here.

Exactly--no relationship and no call for common strings.
Comment 7 Yousuf Philips (jay) (retired) 2017-09-30 18:33:15 UTC
(In reply to V Stuart Foote from comment #6)
> Yes not commonly needed in Impress, and default for both Draw and Impress
> documents really should be the 0 of None.

Would have to disagree about the 0 margins by default for Draw, as similar software (Visio, Publisher) do have margins on by default.
Comment 8 Heiko Tietze 2017-09-30 20:11:36 UTC
Seems none of you reads my comments. So once again.

#1 Labels are inconsistent to local settings
   (inch in dropdown, centimeter in dialog as defined in tools > options)
#2 Inconsistency across modules
   (#1 is true for Writer as well but the content is also different)
#3 Numbers are misleading
   (Draw/Impress uses full size = 2x margin, Writer not = #2)

My suggestion is in comment 2.
Comment 9 V Stuart Foote 2017-10-01 01:48:11 UTC
(In reply to Yousuf Philips (jay) from comment #7)
> (In reply to V Stuart Foote from comment #6)
> > Yes not commonly needed in Impress, and default for both Draw and Impress
> > documents really should be the 0 of None.
> 
> Would have to disagree about the 0 margins by default for Draw, as similar
> software (Visio, Publisher) do have margins on by default

As does DIA; but GIMP, Inkscape, MS Paint, CorelDraw all set 0 margins--we can do what works best for Draw/Impress. With Impress of course set 0.  

Personally, when I use Draw for diagrams and illustrations the fist thing I do is reset margins to zero and resize the canvas to match the size image I need. 

Also, setting default to 0 rather than trying to localize for canvas size just makes the flow easier and simplifies l10n/i18n.

 (In reply to Heiko Tietze from comment #8)
> Seems none of you reads my comments. So once again.
> 
> #1 Labels are inconsistent to local settings
>    (inch in dropdown, centimeter in dialog as defined in tools > options)

And my point to removing "translatable" from .ui, and use bare US or Metric values. Not an issue for Impress presentations, and Draw illustrations are better served to have fixed and symmetric values since we've eliminated the spin boxes for setting each edge.

My suggestion is to define the additional static values in source and add the missing Metric margins to a complete the listbox (like page/paper sizes) and eliminate _any_ text labeling other than None for zero.
Comment 10 Yousuf Philips (jay) (retired) 2017-10-01 14:55:43 UTC
(In reply to Heiko Tietze from comment #8)
> Seems none of you reads my comments. So once again.



> #1 Labels are inconsistent to local settings
>    (inch in dropdown, centimeter in dialog as defined in tools > options)

The labels won't be changing in the dropdown based on the user's defined, as it requires additional computing for minimal benefit, similar to how they don't in Writer.

> #2 Inconsistency across modules
>    (#1 is true for Writer as well but the content is also different)

As already stated in comment 3, "Margins for writer documents are quite different to margins for draw illustrations, so these are quite irrelevant here." which stuart agreed with in comment 6 saying "Exactly--no relationship and no call for common strings."

> #3 Numbers are misleading
>    (Draw/Impress uses full size = 2x margin, Writer not = #2)

Didn't follow.
Comment 11 Yousuf Philips (jay) (retired) 2017-10-01 15:19:49 UTC
(In reply to V Stuart Foote from comment #9)
> As does DIA; but GIMP, Inkscape, MS Paint, CorelDraw all set 0 margins--we
> can do what works best for Draw/Impress. With Impress of course set 0. 

Yes we have to do what is best for each of the apps and Draw needs margins and Impress doesnt. Draw isnt a raster image editor like GIMP or MS Paint, and isnt a feature-complete vector image editors like Inkscape and CorelDraw, so having no margins wouldnt be the right way to go.

> Personally, when I use Draw for diagrams and illustrations the fist thing I
> do is reset margins to zero and resize the canvas to match the size image I
> need.

The only reason i would use Draw would be to do diagrams, as that is the only thing it is fully capable of doing and we have apps like GIMP and Inkscape that are fully features graphics editors. Our survey also confirmed our users primarily use it for that.

https://design.blog.documentfoundation.org/2016/04/01/the-many-faced-god-part-2-how-libreoffice-draw-is-expected-to-evolve/

> Also, setting default to 0 rather than trying to localize for canvas size
> just makes the flow easier and simplifies l10n/i18n.

Would be better if we had a Tools > Options setting to set what the default margins should be for users who dont like the default.

> And my point to removing "translatable" from .ui, and use bare US or Metric
> values. Not an issue for Impress presentations, and Draw illustrations are
> better served to have fixed and symmetric values since we've eliminated the
> spin boxes for setting each edge.

Not sure its a good idea to exclusively use metric values, as you have users who prefer imperial values, which is why i think we should have it a mix of both.

> My suggestion is to define the additional static values in source and add
> the missing Metric margins to a complete the listbox (like page/paper sizes)
> and eliminate _any_ text labeling other than None for zero.

I'm fine with eliminating the text part, but i'm assuming 'cm' would still need to be translated.
Comment 12 V Stuart Foote 2017-10-01 16:29:52 UTC
(In reply to Yousuf Philips (jay) from comment #11)
> 
> > Also, setting default to 0 rather than trying to localize for canvas size
> > just makes the flow easier and simplifies l10n/i18n.
> 
> Would be better if we had a Tools > Options setting to set what the default
> margins should be for users who dont like the default.
> 

Agree, but still think a default margin of 0 would be more functional.

> > And my point to removing "translatable" from .ui, and use bare US or Metric
> > values. Not an issue for Impress presentations, and Draw illustrations are
> > better served to have fixed and symmetric values since we've eliminated the
> > spin boxes for setting each edge.
> 
> Not sure its a good idea to exclusively use metric values, as you have users
> who prefer imperial values, which is why i think we should have it a mix of
> both.
> 

Sorry, I was not clear. The listbox would contain a static set of _both_ metric and US Imperial margins. 

Would require a change to Vidhey's new PageMarginUtils.hxx to define the additional metric values, expanding to have both Metric and US values. Then adust the listbox selections in SlideBackground and the .ui configuration to hold a full selection of all the fixed margin settings.  The US measures are all defined, would need to add the metric values as in comment 5.

> > ... and add
> > the missing Metric margins to a complete the listbox (like page/paper sizes)
> > and eliminate _any_ text labeling other than None for zero.
> 
> I'm fine with eliminating the text part, but i'm assuming 'cm' would still
> need to be translated.

Maybe I'm wrong, but would think both the '"' mark for US, and 'cm' for SI would be universal and need no translation.
Comment 13 Adolfo Jayme 2017-10-02 21:12:10 UTC
(In reply to V Stuart Foote from comment #12)
> Maybe I'm wrong, but would think both the '"' mark for US, and 'cm' for SI
> would be universal and need no translation.

Just a tiny remark: this would need to be translatable for languages not using the Latin script — in Cyrillic, for instance, it becomes “см”. As for the inch mark, we should for once stop using ASCII there; the correct character is U+2033 ″. This one may or may not be translatable; I imagine localizers wanting to change this to e.g. “pulg.” or “in” instead.
Comment 14 V Stuart Foote 2017-10-02 22:14:22 UTC
(In reply to Adolfo Jayme from comment #13)
> Just a tiny remark: this would need to be translatable for languages not
> using the Latin script — in Cyrillic, for instance, it becomes “см”. As for
> the inch mark, we should for once stop using ASCII there; the correct
> character is U+2033 ″. This one may or may not be translatable; I imagine
> localizers wanting to change this to e.g. “pulg.” or “in” instead.

OK, it makes sense leave the .ui translatable for localization of all the margins in the listbox.

Agree using the more correct U+2033 Double Prime: ″ rather than the U+0022 Quotation Mark: " is a good idea.
Comment 15 Commit Notification 2017-10-03 20:36:02 UTC
Katarina Behrens committed a patch related to this issue.
It has been pushed to "master":

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

tdf#109100: Hide margine control in Impress

It will be available in 6.0.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 16 Yousuf Philips (jay) (retired) 2017-10-05 06:47:35 UTC
(In reply to V Stuart Foote from comment #12)
> Agree, but still think a default margin of 0 would be more functional.

Guess we'll agree to disagree. Bug 112888 has the default margins enhancement.

> Sorry, I was not clear. The listbox would contain a static set of _both_
> metric and US Imperial margins. 
> 
> Would require a change to Vidhey's new PageMarginUtils.hxx to define the
> additional metric values, expanding to have both Metric and US values. Then
> adust the listbox selections in SlideBackground and the .ui configuration to
> hold a full selection of all the fixed margin settings.  The US measures are
> all defined, would need to add the metric values as in comment 5.

Yes a mix of the two is what i've also suggested. I think 4 entries per system is good.

0.5cm, 1cm, 1.5cm, 2cm
0.25" (0.635cm), 0.5" (1.27cm), 0.75" (1.9cm), 1" (2.54cm)

> Maybe I'm wrong, but would think both the '"' mark for US, and 'cm' for SI
> would be universal and need no translation.

In the Arabic UI, '"' isnt translated but 'cm' is.
Comment 17 Yousuf Philips (jay) (retired) 2017-10-08 09:56:53 UTC
(In reply to Adolfo Jayme from comment #13)
> As for the inch mark, we should for once stop using ASCII there; the correct
> character is U+2033 ″.

I'm assuming we use the ascii character because users can type the inches value in that way into the field, even if the field is set to a different measurement unit by the user settings.
Comment 18 V Stuart Foote 2017-10-08 13:08:25 UTC
(In reply to Yousuf Philips (jay) from comment #17)
> (In reply to Adolfo Jayme from comment #13)
> > As for the inch mark, we should for once stop using ASCII there; the correct
> > character is U+2033 ″.
> 
> I'm assuming we use the ascii character because users can type the inches
> value in that way into the field, even if the field is set to a different
> measurement unit by the user settings.

Yes, guess that could be an issue. 

For listboxes, as now done here, the "PRIME" and "DOUBLE PRIME" _would_ be more correct string labels. 

But with comboboxes and spinboxes the "APOSTROPHE" and "QUOTATION MARK" actually toggle the field unit during value entry, i.e. to be FUNIT_FOOT and FUNIT_INCH respective. Of course no convenient way to enter the PRIME, DOUBLE PRIME from keyboard so could not add those as unit values [refs].

But, would think risk of using the "correct" characters for the string label is minimal, without worrying too much about departure from the corresponding fieldmetric usage. In fact maybe the combobox and spinbox should labels should shift to in and ft (and match the SI value) rather than the ASCII " and ' there now?

=-ref-=

[1] https://opengrok.libreoffice.org/xref/core/vcl/inc/units.hrc
[2] https://opengrok.libreoffice.org/xref/core/include/vcl/field.hxx#458
[3] https://opengrok.libreoffice.org/xref/core/vcl/source/control/field.cxx#1061
Comment 19 Yousuf Philips (jay) (retired) 2017-10-10 11:26:52 UTC
Created attachment 136890 [details]
margin label in impress

(In reply to Commit Notification from comment #15)
> Katarina Behrens committed a patch related to this issue.
> It has been pushed to "master":
> 
> http://cgit.freedesktop.org/libreoffice/core/commit/
> ?id=6f581504b77cf780898ffb568a66d4aa86df1c73
> 
> tdf#109100: Hide margine control in Impress

@Bubli: The control is hidden but the label is still present.
Comment 20 Xisco Faulí 2017-11-08 23:38:33 UTC
@Bubli, should the label be deleted as well?
Comment 21 Commit Notification 2017-12-27 22:05:37 UTC
Aron Budea committed a patch related to this issue.
It has been pushed to "master":

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

tdf#109100: Hide label for margin control in Impress

It will be available in 6.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 22 Commit Notification 2017-12-29 15:51:36 UTC
Aron Budea committed a patch related to this issue.
It has been pushed to "libreoffice-6-0":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=4eeb0f5a76a6e9190fd1db457042720c10ef804a&h=libreoffice-6-0

tdf#109100: Hide label for margin control in Impress

It will be available in 6.0.0.2.

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 23 Xisco Faulí 2018-01-29 09:05:28 UTC
A polite ping to Aron Budea: is this bug fixed? if so, could you
please close it as RESOLVED FIXED ? Thanks
Comment 24 Xisco Faulí 2018-05-02 09:28:43 UTC
Dear Vidhey PV,
This bug has been in ASSIGNED status for more than 3 months without any
activity. Resetting it to NEW.
Please assigned it back to yourself if you're still working on this.
Comment 25 Roman Kuznetsov 2018-06-29 19:27:52 UTC
IMHO this bug should be closed with FIXED, because there is margin drop-down list on SideBar in Draw/Impress 6.1 beta 2
Comment 26 Yousuf Philips (jay) (retired) 2018-07-06 18:32:48 UTC
The implementation was done but the correct presets are still not added.
Comment 27 Heiko Tietze 2018-07-06 20:38:31 UTC
(In reply to Yousuf Philips (jay) (retired) from comment #26)
> The implementation was done but the correct presets are still not added.

This wont work. You cannot expect devs to read tickets having a patch and 26 comments for missing pieces. I suggest to file another ticket.