Contents items expanding issue
ClosedPublic

Authored by i.Dark_Templar on Oct 21 2018, 1:29 PM.

Details

Summary

There is contents tree on left side of application window of Khelpcenter.

If you select "Single click to open files and directories" in application "System Settings" in menu "Input devices" -> "Mouse", upon single click on any expandable item in contents (let's for example take "Application Manuals" or "System Settings Modules") it expands, upon double click it swiftly expands and collapses back. If it's already expanded, it collapses, and on double click it collapses and expands back.

However, if you select "Double click to open files and directories (select icons on first click)" in "System Settings", you no longer can expand/collapse any expandable items in Khelpcenter's contents tree using mouse clicks. You need to double click to expand it, and in best case you can see it swiftly expanding and collapsing back if it's collapsed, or vice versa if it's expanded.

It looks like with "Double click" setting selected contents item receives two events on double click: "selected" and "double click". On first event "expanded/collapsed" state is toggled via explicit calls to corresponding functions (see function Navigator::slotItemSelected), on second event this state is toggled again, thus reverting it to original state. This patch sets a specific setting to get rid of second event and make behaviour similar no matter which mouse settings is used, "single click" or "double click".

BUG: 399275

Diff Detail

Repository
R125 KHelpCenter
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
i.Dark_Templar created this revision.Oct 21 2018, 1:29 PM
Restricted Application added a project: Documentation. · View Herald TranscriptOct 21 2018, 1:29 PM
Restricted Application added a subscriber: kde-doc-english. · View Herald Transcript
i.Dark_Templar requested review of this revision.Oct 21 2018, 1:29 PM

There is already a proposed fix for this, with different code, which I could not test yet:
https://phabricator.kde.org/D16057

What do you think of that other fix?

I've seen it. Linked change fixes different issue:

If you open in the contents tree item "KInfoCenter Modules" and then any appearing item (let's take for example "Device Viewer"), a subtree appears in contents tree for opened item ("Device Viewer").

However, if you open those generated items, all of the items from appearing subtree are leading to "KInfoCenter" index page instead of corresponding page they are named for. For example try clicking/double clicking "The Default KInfoCenter Modules" (or it's subitem "System Information Module").

If you open "Application Manuals" -> "System" -> "Info Center", same subtree appears there, but if you click same mentioned items, they open corresponding pages instead of "KinfoCenter" index page. I mean, if you try opening "System Information Module" subitem, "System Information Module" page is opened.

It looks like at those items that don't work correctly, URL is malformed. It looks like "help:/kinfocenter#kcm/something" or "help:/kinfocenter#memory/something" instead of "help:/kinfocenter/something" (I don't remember exact URLs, but it looks something like this).

Malformed URL is generated by taking URL from "KInfoCenter" subitems which looks like "help:/kinfocenter#kcm" and so on, and just appending page address, which forms as already mentioned "help:/kinfocenter#memory/something" (page "help:/kinfocenter", fragment "memory/something"). To fix it the fragment "#kcm" (and others) have to be stripped when such URL is formed. And it would result into "help:/kinfocenter/something" instead.

And as you may see, after applying linked change, there will be multiple "KInfoCenter" subtrees in contents tree. In order to merge them into one subtree (and thus get rid of duplicates), there's one more change proposed: https://phabricator.kde.org/D16308

If the other change fixes a different issue, but it links to the same bug, it means that either this bug is not related to the issue fixed by this bug, or that the bug should be split in two.

This is change for bug 399275.

For https://phabricator.kde.org/D16308 I see bug 399274 (different bug).

For https://phabricator.kde.org/D16057 I don't see any linked bug.

Am I missing something? Is it linked to a bug somehow and I can't see it?

It's not linked only because the commit message was not cleaned. But if you check the first lines of
https://phabricator.kde.org/D16057

they currently read:
Fixed bug "Khelpcenter menu does not work (some parts of TreeView not expanded)"
which is exactly the content of
https://bugs.kde.org/show_bug.cgi?id=399275

Oh, I see the source of confusion now. I think https://phabricator.kde.org/D16057 shouldn't link to bug 399275 since description of that bug is describing contents index items not expanding/collapsing if "double click" is selected if I understood it correctly.

Alternatively, I can remove link from this change to bug 399275.

The review that fixes a bug should have the BUG: entry, so it's all up to whether that review and that bug are really linked. They have the same author, so let's hear from @pavelmos

The review that fixes a bug should have the BUG: entry, so it's all up to whether that review and that bug are really linked. They have the same author, so let's hear from @pavelmos

@i.Dark_Templar and I discussed this problem. These patches fix various issues.
D16352 fix bug 399275 - That's right!
D16308 fix bug 399274.
D16057 - this patch fixes a bug that is not present in the bugtracker (bugs.kde.org).

I corrected the descriptions of the patches.

Are there any obstacles remaining which prevent from merging these changes? It'd be great if you could review them and merge. If something still prevents the merge, I'd be glad to help fixing that.

ltoscano accepted this revision.Nov 19 2018, 12:46 AM

Sorry for the late review; thank you!

This revision is now accepted and ready to land.Nov 19 2018, 12:46 AM
This revision was automatically updated to reflect the committed changes.