Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

View menu for sidebar option can be out of sync with sidebar state #8115

Closed
2 tasks done
evanhorn opened this issue Oct 3, 2021 · 3 comments · Fixed by #8202
Closed
2 tasks done

View menu for sidebar option can be out of sync with sidebar state #8115

evanhorn opened this issue Oct 3, 2021 · 3 comments · Fixed by #8202
Labels
bug Confirmed bugs or reports that are very likely to be bugs ui

Comments

@evanhorn
Copy link

evanhorn commented Oct 3, 2021

JabRef version

Latest development branch build (please note build date below)

Operating system

GNU / Linux

Details on version and operating system

Ubuntu 20.04, Gnome desktop

Checked with the latest development build

  • I made a backup of my libraries before testing the latest development version.
  • I have tested the latest development version and the problem persists

Steps to reproduce the behaviour

  1. Select one of the sidebar options (i.e., Groups interface) from the View menu (alternatively, use the shortcut key)
  2. Close the sidebar by clicking on the “X” in the top right corner
  3. Open View menu and observe that the sidebar option has a checkmark next to it.
  4. Select same sidebar option (with checkmark)
  5. Open View menu and observe that the sidebar option checkmark has disappeared even though the sidebar option is active.

Appendix

No response

@ThiloteE
Copy link
Member

ThiloteE commented Oct 4, 2021

Can reproduce this on

JabRef 5.4--2021-08-21--644e48d
Windows 10 10.0 amd64
Java 16.0.2
JavaFX 16+8

and

JabRef 5.4--2021-09-20--a91420d
Windows 10 10.0 amd64
Java 16.0.2
JavaFX 17+18

@Siedlerchr Siedlerchr added bug Confirmed bugs or reports that are very likely to be bugs ui labels Oct 4, 2021
@Siedlerchr
Copy link
Member

Might also be related to #8082

@evanhorn
Copy link
Author

evanhorn commented Oct 4, 2021

I don't think it is directly related to #8082. I observed the behavior detailed in #8082 after I submitted this issue, but I have not observed it for any of the previous builds. However, I have observed the behavior that I described for some time and in numerous previous versions. Since it was more cosmetic than an interference (and I was lazy), I didn't submit an issue until I got on here to report a more serious issue.

Siedlerchr pushed a commit that referenced this issue Dec 14, 2021
* Add visibleProperty to SidePaneComponent

* Making progress into refactoring Sidepane logic

- Add immutable info to SidePaneType enum
- Replace SidePaneComponent by SidePaneView which has a SidePaneHeaderView that contains all input handling
- Create SidePaneContainerView the replacement for SidePane

* Populate SidePaneType with information

* Wrap the content of web search side pane into WebSearchPaneView

* Revert changes to SidePane

* Refactor SidePaneContainer

- Move some state and logic to SidePaneContainerViewModel
- Implement the moveUp and moveDown action
- Implement lazy loading for SidePaneView
- Expose some initial API for fixing #8115

* Bind side pane check menu selection state to side pane visibility

* Fix checkstyle

* Remove SidePaneViewModel.java

* Fix view menu for sidebar option out of sync with sidebar state

- Fix #8115
- Deprecate the old implementation of Sidepane and remove all of its usage in the rest of the application
- Implement the toggle method in SidePaneContainerView

* Remove unused constructor parameters

* Remove deprecated Sidepane component

* Move Vbox.setVgrow() to SidePaneView

* Merge SidePaneHeaderView into SidePaneView

* Avoid firing removed and added events on ObservableList

- Swapping elements in ObservableLists is rather difficult to handle. The problem with this code here is that two events are fired (one for removing, and on for adding). Perhaps the easiest solution is to use the sort method using a custom comparator. Since the list is small we can also be inefficient and do this as follows: create a copy of visiblePanes, swap the elements there and then use this new list as a reference for the comparator

* Use shorter more declarative code for binding

* Rename SidePaneView to SidePaneComponent and SidePaneContainerView to SidePane

* Rename initView() to initialize()

* Add an overloading of createCheckMenuItem() with a selected property

* Rename sidePaneVisibleProperty() to paneVisibleProperty()

* Use the name pane rather than sidePane to call components in the SidePane

* Remove unused method createSidePaneCheckMenuItem()

* Share the visibility property of side pane components by moving it to StateManager

* Remove comment

* Use better names

* Fix checkstyle

* Move close/toggle pane actions to separate files

* Move sidepane binding logic to SidePaneViewModel

* Removed unnecessary properties in stateManager, refactored to mvvm pattern

* Moved listener from sidepane visibility property to list of children in sidepane

* Removed unused method

* Fixed sidepane width issue on startup

* Created tests

* Cleanups

* CHANGELOG.md

* more CHANGELOG.md

Co-authored-by: Carl Christian Snethlage <[email protected]>
@koppor koppor moved this to Done in Prioritization Nov 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Confirmed bugs or reports that are very likely to be bugs ui
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants