Symbol Library Editor: Library tree scrollbar position is reset after editing field properties

[Fabián Inostroza]

Open a symbol in the library editor, one that's in the middle the list so that the scrollbar is not at the top, open the field properties dialog and click ok.

The scrollbar should stay at the current position but after clicking ok in the field properties dialog the position is reset.

Edit: The position is also reset when you place a line, circle, text or a pin

Application: kicad
Version: (5.0.0-rc3-dev-2-g149a0f58a), debug build
Libraries:
    wxWidgets 3.0.4
    libcurl/7.60.0 GnuTLS/3.5.18 zlib/1.2.11 libidn2/2.0.4 libpsl/0.20.2 (+libidn2/2.0.4) libssh2/1.8.0 nghttp2/1.32.0 librtmp/2.3
Platform: Linux 4.16.0-2-amd64 x86_64, 64 bit, Little endian, wxGTK
Build Info:
    wxWidgets: 3.0.4 (wchar_t,wx containers,compatible with 2.8) GTK+ 2.24
    Boost: 1.62.0
    OpenCASCADE Community Edition: 6.9.1
    Curl: 7.60.0
    Compiler: GCC 7.3.0 with C++ ABI 1011

Build settings:
    USE_WX_GRAPHICS_CONTEXT=OFF
    USE_WX_OVERLAY=OFF
    KICAD_SCRIPTING=ON
    KICAD_SCRIPTING_MODULES=ON
    KICAD_SCRIPTING_WXPYTHON=OFF
    KICAD_SCRIPTING_ACTION_MENU=ON
    BUILD_GITHUB_PLUGIN=ON
    KICAD_USE_OCE=ON
    KICAD_USE_OCC=OFF
    KICAD_SPICE=OFF

[Fabián Inostroza (fabianinostroza)]

Every time a component is double clicked in the tree it is unselected by code in CMP_TREE_PANE::onComponentSelected,

also, when the tree widget loses focus the component is unselected by code in LIB_EDIT_FRAME::GeneralControl (a call to ClearSearchTreeSelection)

In the end, the current item is unselected with a call to wxDataViewCtrl::Unselect with the following comment: "// Make sure current-part highlighting doesn't get lost in seleciton highlighting".

This unselection of the current item makes the call to wxDataViewCtrl::GetSelection in COMPONENT_TREE::GetSelectedLibId always return an invalid item

[Maciej Suminski (orsonmmz)]

Actually any symbol modification in the Symbol Library Editor causes the scrollbar to reset. This is an issue related to the wxDataViewCtrl widget, where we had to choose between quick destroy-and-rebuild or slow update-existing data strategy.

[Fabián Inostroza (fabianinostroza)]

Hi @Maciej,

The thing is that because of the functions call I noticed in my last comment the method COMPONENT_TREE::Regenerate() is unable to restore the selected component (it only expands the previously expanded trees) because the item being edited gets unselected.

I commented out those function calls and now the tree is still rebuild when editing the symbol but the item is now selected and the scrollbar is at least in a correct place, the symbol is visible in the tree without scrolling.

I don't know why these function call were added in the first place, they seem to be doing the inverse the comment says.

[Fabián Inostroza (fabianinostroza)]

Now I see why that code was added: https://bugs.launchpad.net/kicad/+bug/1741719,

Maybe an intermediate solution would be to store the currently selected footprint inside kicad code and not rely on the wxDataViewCtrl instance.

Anyway, at least with my system libraries and theme I am not affected by the original bug report, watch the attached video.

[Maciej Suminski (orsonmmz)]

I assume the attached video demonstrates code running with the mentioned lines commented out? I think lp:#1741719 might be macOS specific.

I attach the discussed patch for reference, I will test it on Windows. Perhaps the two calls might become macOS specific if they help there.

[Maciej Suminski (orsonmmz)]

@Jeff, @Seth: could you check if the patch in #5 fixes or breaks scrollbar jumping in the Symbol Editor on macOS?

On Windows the scrollbar behaves better with the two lines in place, i.e. there is absolutely no jump when a symbol is modified, whereas with the lines removed - the scrollbar will jump to the modified symbol if it was previously invisible.

The remaining question is shall the mentioned lines be excluded on Linux or Windows specific.

[Fabián Inostroza (fabianinostroza)]

Yes, the previous video was with the lines commented. I've attached a new video showing what happens with the current master, just in case you can't reproduce.

[Jeff Young (jeyjey)]

The issue those lines are trying to fix is one of comprehension.

Double click a symbol to have it placed into the edit canvas.

Now single-click on a different symbol. The highlighting of the just-selected symbol is more visually noticeable than the highlighting for the currently-on-canvas symbol, making it very easy to think that you're editing a different symbol than you actually are.

The scrollbars jumps on OSX to make sure that the symbol-on-canvas highlighting is visible when you're editing the symbol.

The video in #7 certainly appears to show that something different happens on Linux.

[Jeff Young (jeyjey)]

@Orson, your comment in #6 says Windows is better /with/ the lines, but then asks if they should be excluded on both Linux and Windows or just Windows.

I assume one of them is a typo, but I'm not sure which one. (Or maybe I misunderstood it entirely....)

[Maciej Suminski (orsonmmz)]

Jeff,

Agreed, my wording was not very clear. These lines are needed on Windows, and as far I understand macOS also takes advantage of them, so I guess they should be wrapped with #ifndef __WXGTK__..#endif.

Is macOS behavior similar to what Fabian demonstrates on the video in #4? If so, I think it is acceptable and definitely better than the behavior shown on #7 video.

[Jeff Young (jeyjey)]

@Orson, yes, the Mac behaviour with the lines is similar to video #4 (slightly better even, as it only scrolls if the item is out-of-view).

I also concur with #ifndef __WXGTK__..#endif.

[Seth Hillbrand (sethh)]

Agreed. Looks good here. 5.0.1?

[Jeff Young (jeyjey)]

There's no longer any need for this as I've gone with a different solution in 5.1. Hopefully it will work better on the various platforms....

[KiCad Janitor (kicad-janitor)]

Fixed in revision 0a35c5c97eb47c67329a47d49378268ff0f85737
https://git.launchpad.net/kicad/patch/?id=0a35c5c97eb47c67329a47d49378268ff0f85737

[Fabián Inostroza (fabianinostroza)]

Hi Jeff, I tested the changes and it works fine in the footprint editor, the scrollbar doesn't jump however in eeschema the behavior is still the same.

[Jeff Young (jeyjey)]

@Fabian, there have been somme more changes in this area, but since I couldn't ever reproduce it on Mac I'm not sure if they will fix the symbol editor or not. If you're using the Nightlies could you check one more time?

[Fabián Inostroza (fabianinostroza)]

Hi Jeff, the behavior is the same as in my last comment. See the attached video.

If you tell me where to look I can try to debug or try some changes, I don't know much the kicad codebase.

[Jeff Young (jeyjey)]

Yes, that video helped (or something has changed in between). In any case I can reproduce it now.

[KiCad Janitor (kicad-janitor)]

Fixed in revision f9aaa013292e12ae3ac839bf331e2d9adf22192c
https://git.launchpad.net/kicad/patch/?id=f9aaa013292e12ae3ac839bf331e2d9adf22192c