Bug 108999 - UI: uno:WrapThroughTransparent menu checkbox can be enabled, but not disabled
Summary: UI: uno:WrapThroughTransparent menu checkbox can be enabled, but not disabled
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: UI (show other bugs)
Version:
(earliest affected)
Inherited From OOo
Hardware: All All
: medium normal
Assignee: Not Assigned
URL:
Whiteboard: target:6.0.0
Keywords: regression
Depends on:
Blocks: Context-Menu Anchor-and-Text-Wrap
  Show dependency treegraph
 
Reported: 2017-07-07 11:41 UTC by Justin L
Modified: 2017-07-13 12:00 UTC (History)
5 users (show)

See Also:
Crash report or crash signature:
Regression By:


Attachments
wrap_inBackground.png: The right-click menu for "Wrap" settings doesn't toggle "In Background" off. (23.11 KB, image/png)
2017-07-07 11:41 UTC, Justin L
Details
tdf77219_foregroundShape.docx: sample document for toggling "In Front of Text" and "Behind Text" (10.31 KB, application/vnd.openxmlformats-officedocument.wordprocessingml.document)
2017-07-07 11:57 UTC, Justin L
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Justin L 2017-07-07 11:41:21 UTC
Created attachment 134528 [details]
wrap_inBackground.png:  The right-click menu for "Wrap" settings doesn't toggle "In Background" off.

"In Background" checkbox can only be turned on.  (That is because it actually coded as a radio button - switching between wrapthru and wrapthru_transp.)

The UI for placing something in the background is confusing because:
-in context menus, the icon is a checkbox, but it functions as a radio button
-it isn't obvious that "in background" is the opposite of "wrap through"
-it doesn't act like the checkboxes below it like "First paragraph"
-in the full dialog boxes, it is actually implemented as a checkbox toggle.

Inherited from OOo.
suggested fix: https://gerrit.libreoffice.org/39673 - use FRAME_WRAPTHRU_TRANSP as transparency toggle
Comment 1 Justin L 2017-07-07 11:57:19 UTC
Created attachment 134529 [details]
tdf77219_foregroundShape.docx: sample document for toggling "In Front of Text" and "Behind Text"
Comment 2 Heiko Tietze 2017-07-07 21:11:11 UTC
From gerrit: "Gerrit is not a good place to discuss issues; bug 59326, bug 64445, and bug 71920 looks interesting but a new ticket would also be good. Unfortunately I cannot build at the moment for some reasons so it's hard to follow your on/off idea of wrapthrough. If its possible to set warpleft plus through it makes not much sense. The difference between through and background becomes clear when the background is set with some transparency. The frame becomes opaque, and we could do that as default. Just an idea. The checkbox issue is clear, big mess the whole thing. Ideally we improve the 'preview' (actually different icons today) to show how the various combination of settings look eventually. I would remove the advanced options from the quick access (e.g. have only one through option in toolbar, sidebar, context menu etc.). If you plan to revamp wrapping completely we can think about better solution from the shell. Some time ago I was talking with Jay about the topic having a quick poll in mind https://docs.google.com/document/d/12pQZg6TAiDubPhVwxtqs1rD4vmXMbZKvOBtskY_maGs/"

Was able to compile meanwhile and the patch toggles wrap through with in background as required from the function. Great work, Justin!
Comment 3 Yousuf Philips (jay) (retired) 2017-07-09 18:14:19 UTC
(In reply to Justin L from comment #0)
> "In Background" checkbox can only be turned on.  (That is because it
> actually coded as a radio button - switching between wrapthru and
> wrapthru_transp.)

It used to be a radio button in 5.1, so this is a regression introduced in 5.2, as all the entries in the Wrap submenu should be radio buttons.
Comment 4 Justin L 2017-07-10 21:32:57 UTC
Committed in LO 6.0 master by commit
https://cgit.freedesktop.org/libreoffice/core/commit/?id=959be1b5a9cd522394dba9366686a1256588223b

UI Wrap InBackground: transition to wrapthru transparency toggleHEADmaster
This patch creates FN_FRAME_WRAPTHRU_TOGGLE to replace
most uses of the previous function.

Change-Id: Ia46ddbd47899e8ca569bf3adb2b1c4ad7cfa1920
Comment 5 Xisco Faulí 2017-07-12 09:08:37 UTC
(In reply to Yousuf Philips (jay) from comment #3)
> (In reply to Justin L from comment #0)
> > "In Background" checkbox can only be turned on.  (That is because it
> > actually coded as a radio button - switching between wrapthru and
> > wrapthru_transp.)
> 
> It used to be a radio button in 5.1, so this is a regression introduced in
> 5.2, as all the entries in the Wrap submenu should be radio buttons.

So, it was changed from radiobutton to checkbutton (visually speaking) in this commit:

author	Maxim Monastirsky <momonasmon@gmail.com>	2016-01-18 22:37:47 (GMT)
committer	Maxim Monastirsky <momonasmon@gmail.com>	2016-01-19 00:05:01 (GMT)
commit 447c313586e9b36acff393feae15f5e1b63861ae (patch)
tree 2cbfc56c4aa843351261731e5ac43d1f59104650 /sw/uiconfig
parent 8cd6dd0a35b1c531ddb5010a3dea84f91f2ffb40 (diff)
tdf#93837 Convert Writer context menus to xml

@Maxim: Was it deliberated to change the submenu from radiobutton to checkbutton?
Comment 6 Maxim Monastirsky 2017-07-13 12:00:22 UTC
(In reply to Xisco Faulí from comment #5)
> @Maxim: Was it deliberated to change the submenu from radiobutton to
> checkbutton?
Unlikely. I guess I just copy-pasted that submenu from menubar.xml, which never had those entries as radio buttons. It should be easy to fix, by adding menu:style="radio" to all affected entries. (And feel free to do that, as I'm not involved with LO development at this stage, due to other commitments.)