STR: 1. Open Options >> LibreOffixe >> Fonts 2. Add an entry to the replacement table 3. Click the entry multiple times Result: First checkbox is toggled on each click. Expected result: Checkbox doesn't toggle. Regression from: commit 2471d6f44c7e8ecbe86a90eeb593b899a08a7408 Author: Attila Szűcs <szucs.attila3@nisz.hu> Date: Fri Oct 16 09:54:14 2020 +0200 tdf#116675 vcl tree list: toggle by label click (e.g. in AutoFilter) BTW: this change is generic VCL code. No idea, why the commit says: Note: Implemented only for generic VCL plugin. Testing on Linux: SAL_USE_VCLPLUGIN=gen instdir/program/soffice
Was that from merging treelist in vcl/toolbox? And then this for bug 116675? https://gerrit.libreoffice.org/c/core/+/104414
(In reply to V Stuart Foote from comment #1) > Was that from merging treelist in vcl/toolbox? And then this for bug 116675? > > https://gerrit.libreoffice.org/c/core/+/104414 I don't think so. Bug 116675 is an enhancement, no bug fix. I found this when working on bug 138857 and did a bibisect to pinpoint the commit. If that is your question?
Sorry, guess what I meant was that it was intentional for handling treelist items with checkbox and labels, that selecting the label should select the checkbox. It didn't use to. But IIUC since the Treelists were all welded and "consolidated" into vcl/treelist dialogs with treelist checkboxes and list items like the Font replacment table are also going to have the full entry selected including the first checkbox as here. We saw this first with bug 138665 for the Tools -> Options dialog. Not surprising it can affect more locations. https://gerrit.libreoffice.org/c/core/+/100498 https://gerrit.libreoffice.org/c/core/+/100591
(In reply to V Stuart Foote from comment #3) > Sorry, guess what I meant was that it was intentional for handling treelist > items with checkbox and labels, that selecting the label should select the > checkbox. It didn't use to. Yes. That is bug 116675. Sure that fix was intentional. But the check boxes in that table have no label. I didn't look at the autofilter implementation to see, if these labels are actually implemented as a 2nd column. The font replacement table has colums just with checkboxes without any labels. > But IIUC since the Treelists were all welded and "consolidated" into > vcl/treelist > dialogs with treelist checkboxes and list items like the Font replacment > table are also going to have the full entry selected including the first > checkbox as here. > > We saw this first with bug 138665 for the Tools -> Options dialog. Not > surprising it can affect more locations. Sorry, but now I'm even more confused, what your actual comments are about. Bug 138665 is about visual tree indicators... how does that come in here? My bug is very simple. I click somewhere on my table row to select it. It has four columns. Two checkboxes, two text columns. Every time the row is clicked anywhere, the leading check box is toggled. FWIW, the tree view just displays a table, no actual tree, but the internal class is SvTreeListBox. Maybe the bug title need adoptions, but it's a regression bibisected to this commit and I even did a local revert, so I could easier fix my bug.
I think it is a design question. If you can find a way how it should work, that most peoples agree with, i'm gladly try to change the work of it to that way. I know it is not ideal this way.. but im not sure what would be the best.. [and not too complex :)] in the bug 116675, it was asked to make checkbox easyer toggleable.. because the checkbox is small... hard to click. so i made that the whole row (-buttons) toggle the checkbox with left click.. [but it is happening only if the .UI file is set like that... in options -> load/save -> Microsoft office ... there is a list with 2 checbox/row.. that not toggle with clicking on the row] You can still right click on it.. to get the focus without toggle.. and use other way to select... like arrow keys.. but i know it is just a workaround.. i tried to make that only Label text toggle the checkbox.. but text can be much smaller then the checkbox.. especially if it is '1' and there was other annoyances.. like there is space between the text and the checkbox, where click would not toggle.. and the cases when there is more columns with texts in the rows.. the MSO excel is not a good example here.. because in autofilter you can select the row only by clicking the text or checkbox... but there it toggles the checkbox too.. [however excel increase the selection with a few pixels.. it is a bit easyer to click] so in excel have the same problem, but harder to click as in calc. an other difference is that excel selection [the blue rectangle] is only around the text.. while in calc the selection is around the whole row... it may seems not relevant.. but this way we get a hint where we can click to toggle/select.. [we see visually the selection rectangle size] if we have to click on the textbox to toggle, but the selection is the whole row.. then it is a bit confusing to figure out whitch pixel's click will toggle and witch will select... and if the texts are full row wide, then again we can't select without toggle... I was thinking about more complex solution too, but im not a designer.. like: -we could change that right click would toggle, and left click select -or it could toggle only if the clicked row is already selected.. so you could select with 1 click, and toggle with 2 clicks.. -or we could give a minimum area (several letter wide) where clicking would toggle..
(In reply to Attila Szűcs from comment #5) > I think it is a design question. No it's not. It's a bug. The checkboxes in that table don't have an associated label. Please look at that table. The row consists of: [ ] [x] font1 font2 It not like the first checkbox has any relation to the selection of the row. > If you can find a way how it should work, that most peoples agree with, i'm > gladly try to change the work of it to that way. > > I know it is not ideal this way.. but im not sure what would be the best.. > [and not too complex :)] > > in the bug 116675, it was asked to make checkbox easyer toggleable.. because > the checkbox is small... hard to click. > > so i made that the whole row (-buttons) toggle the checkbox with left > click.. [but it is happening only if the .UI file is set like that... Now that would be an easy fix. I had a look at the UI files, but for me it looks the same for cui/uiconfig/ui/optfontspage.ui and cui/uiconfig/ui/optfltrembedpage.ui, if this is an easy UI file feature, then please fix it. > in options -> load/save -> Microsoft office ... there is a list with 2 > checbox/row.. that not toggle with clicking on the row] I understand this, but in this case the label is associated with the checkbox. > You can still right click on it.. to get the focus without toggle.. and use > other way to select... like arrow keys.. but i know it is just a workaround. It's totally unexpecting for a table that selecting a row toggles a checkbox, especially since the row contains multiple checkboxes. Actually a user might even miss the "toggle on select" behavior, which will be even more confusing later. > [more text about the Excel case you want to emulate] This is all fine for your special case, where the row is represented by the checkbox. It's also the same for stand alone checkboxes with label, where you can also click the label to toggle the checkbox. > I was thinking about more complex solution too, but im not a designer.. > > like: > -we could change that right click would toggle, and left click select > -or it could toggle only if the clicked row is already selected.. so you > could select with 1 click, and toggle with 2 clicks.. > -or we could give a minimum area (several letter wide) where clicking would > toggle.. This is all unexpected and it's not needed, really. Everything is fine for you special Autofilter case, but nevertheless your code somehow broke the font replacement table.
(In reply to Jan-Marek Glogowski from comment #6) > (In reply to Attila Szűcs from comment #5) > > I think it is a design question. > > No it's not. It's a bug. The checkboxes in that table don't have an > associated label. > Please look at that table. The row consists of: > > [ ] [x] font1 font2 > > It not like the first checkbox has any relation to the selection of the row. >... > > This is all unexpected and it's not needed, really. Everything is fine for > you special Autofilter case, but nevertheless your code somehow broke the > font replacement table. OK, but... is the Screen Only CB actually doing anything? I've been trying to work with it but it seems to have no effect, at least on the Windows builds. Just the 'Always' CB seems to function. And, shouldn't they be mutually exclusive CB, one or the other but not both?
(In reply to V Stuart Foote from comment #7) > (In reply to Jan-Marek Glogowski from comment #6) > > (In reply to Attila Szűcs from comment #5) > > > I think it is a design question. > > > > No it's not. It's a bug. The checkboxes in that table don't have an > > associated label. > > Please look at that table. The row consists of: > > > > [ ] [x] font1 font2 > > > > It not like the first checkbox has any relation to the selection of the row. > > >... > > > > This is all unexpected and it's not needed, really. Everything is fine for > > you special Autofilter case, but nevertheless your code somehow broke the > > font replacement table. > > OK, but... is the Screen Only CB actually doing anything? I've been trying > to work with it but it seems to have no effect, at least on the Windows > builds. Just the 'Always' CB seems to function. And, shouldn't they be > mutually exclusive CB, one or the other but not both? Please don't try to hijack this bug report. This is just about broken GUI behavior, since the bibisected commit was merged. Please consult the help page to understand, what these checkboxes actually mean, to be sure (see https://help.libreoffice.org/7.0/en-US/text/shared/optionen/01010700.html) and then verify, if they really don't work as expected and eventually open a new bug report. After reading comment 5 I compared the UI files of the working and broken example to no avail. Than I checked out again commit cd384e2d31f74223948ea70d8aa3c318d3ceeb50 but now especially the changes to cui/source/options/fontsubs.cxx, so now there is https://gerrit.libreoffice.org/c/core/+/107920, which seems to fix the problem.
(In reply to Jan-Marek Glogowski from comment #8) > Please don't try to hijack this bug report. This is just about broken GUI > behavior, since the bibisected commit was merged. Please consult the help > page to understand, what these checkboxes actually mean, to be sure (see > https://help.libreoffice.org/7.0/en-US/text/shared/optionen/01010700.html) > and then verify, if they really don't work as expected and eventually open a > new bug report. And just to prevent any false impression here. I worked on tdf#138857, because of a user post to our https://ask.libreoffice.org w.r.t. font replacement. I misunderstood the column names and so did the user. And then I discovered the broken UI behavior of that table described in tdf#138857, fixed it and asked UX for a review of that table GUI, which UX simply closed. IMHO the "checkbox explain" table from help should be beneath this table in the dialog, like the explanations for the checkboxes in "options -> load/save -> Microsoft office". Or at least some extended help / tooltip for the table. But this is also not the point of this bug :-)
(In reply to Jan-Marek Glogowski from comment #9) I understand your problem, and i agree that this is a bug. The question is how to 'fix' it .. on witch level of the program... or how should it work We can fix it where the font replacement table work is coded. Or we can try to fix it in much deeper place, where SvTreeListBox coded. In tdf#116675, the deeper part was asked to be modified, so they asked that by dedault all the TreeList should toggle their checkbox. Autofilter was just a good example there. In other words the question is.. how should Treelist generally work.. and what should we code by specializing the actual Treelist code, like your table.. Because i think the general working may not well designed/decided.. and if it will change later.. that may require to rewrite many special cases as well. That is why i asking about how should the genral, default case work.. About the .UI file.. i was not sure where it came from.. [i'm not really experienced in LO .. i'm working here only like half a year] but during debug, i think i seen that Treelist store somehow that it have a special checkbox.. that i toggled.. or not.. but beside that it could have any amount of other checkboxes. But i didnt traced down where this information come from... from the .UI file.. or from the core around where the .UI file loaded..
Maybe... a) the auto-toggle on click should only kick in if there is only one checkbox in a row (which would solve this case regardless of any other potential changes) because how do we know which one should toggle. b) it seems more natural to me that the first click on an unselected row just selects it. Subsequent clicks on a selected row to change the toggle like it currently does seems fairly ok too. Another option is that this toggle on single click is something that really only the autofilter wants to do, it might then be the case that setting "single-click-activate" on the treeviews in filterdropdown.ui to true (which sets mbActivateOnSingleClick in SvTreeListBox) then connecting via connect_row_activated to know when a row was clicked on and manually toggle the row's checkbox in a callback there would work and make this auto-toggling on row click an autofilter only feature. and/or some hybrid of b where if mbActivateOnSingleClick is set then a first click on an unselected row *does* toggle and select it, but with mbActivateOnSingleClick false it just selects it.
Jan-Marek Glogowski committed a patch related to this issue. It has been pushed to "master": https://git.libreoffice.org/core/commit/4cabfc30bf2db873930cef4f7f1b0243ae2fde28 tdf#138865 don't set ColumnToggleType::Check It will be available in 7.2.0. The patch should be included in the daily builds available at https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: https://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
Jan-Marek Glogowski committed a patch related to this issue. It has been pushed to "libreoffice-7-1": https://git.libreoffice.org/core/commit/ee601c71908dc05fb0f3bd6c4916041d889e96e2 tdf#138865 don't set ColumnToggleType::Check It will be available in 7.1.0.2. The patch should be included in the daily builds available at https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: https://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
László Németh committed a patch related to this issue. It has been pushed to "master": https://git.libreoffice.org/core/commit/6f6922fb467945157b9a5b42dea6f7e0ef2f8685 Revert "tdf#138865 don't set ColumnToggleType::Check" It will be available in 7.2.0. The patch should be included in the daily builds available at https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: https://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.