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

JavaFX FileAnnotationTab #3082

Merged
merged 26 commits into from
Aug 16, 2017
Merged

JavaFX FileAnnotationTab #3082

merged 26 commits into from
Aug 16, 2017

Conversation

LinusDietz
Copy link
Member

@LinusDietz LinusDietz commented Aug 7, 2017

I have created an initial port of the FileAnnotationTab. It is not done yet, as I am not so proficient in GUI programming.
Fixes #2922
Fixes #3059

  • Test if multiple linked files are working
  • Add Reload Annotations button
  • Add Copy To Clipboard button
  • Check if this PR fixes Switching from FileAnnotationTab causes high CPU load #3059
  • Remove unused variables
  • Is it possible to get a nicer binding? At the moment we have 5 StringProperties instead of one ObjectProperty
  • fix NPE when clicking Copy annotations, if there are no annotations
  • Fix reload annotaitons button
  • Add FileAnnotationsTab when adding a file (Without reopening the Entry Editor)
  • Fix design glitches (first line of right side)
  • Fix annotation auto-selection when switching between linked files


import static org.jabref.model.pdf.FileAnnotationType.NONE;

public class FxFileAnnotationTab extends EntryEditorTab {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why don't you use the Scence Builder to create your fxml?
http://gluonhq.com/open-source/scene-builder/

LinusDietz and others added 6 commits August 8, 2017 10:53
…avafx-fileannotationtab

# Conflicts:
#	src/main/java/org/jabref/gui/entryeditor/FileAnnotationListCellRenderer.java
#	src/main/java/org/jabref/gui/entryeditor/FxFileAnnotationTab.java
@tobiasdiez
Copy link
Member

So, I just added the final touch. Here it how it looks now:
image

(no idea why the "Author" label is positioned at the bottom of the row...if somebody wants to try he or she is invited to fix that.)

@tobiasdiez tobiasdiez added status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers and removed status: help wanted labels Aug 8, 2017
@tobiasdiez tobiasdiez changed the title [WIP] JavaFX FileAnnotationTab JavaFX FileAnnotationTab Aug 8, 2017
@lenhard
Copy link
Member

lenhard commented Aug 9, 2017

I've tried out the PR and found a number of functional issues that need to be addressed before we can merge this:

  1. On first clicking on the file annotation tab in the entry editor, I get the following exception in the log:
Opening: \\nas-a1\redirected$\jorglenh\Desktop\testtest.bib
Uncaught exception Occurred in Thread[JavaFX Application Thread,5,main]
java.lang.ClassCastException: com.sun.javafx.stage.EmbeddedWindow cannot be cast to javafx.stage.Stage
	at org.jabref.gui.AbstractView.lambda$getView$2(AbstractView.java:34)
	at com.sun.javafx.binding.ExpressionHelper$SingleChange.fireValueChangedEvent(ExpressionHelper.java:182)
	at com.sun.javafx.binding.ExpressionHelper.fireValueChangedEvent(ExpressionHelper.java:81)
	at javafx.beans.property.ReadOnlyObjectPropertyBase.fireValueChangedEvent(ReadOnlyObjectPropertyBase.java:74)
	at javafx.beans.property.ReadOnlyObjectWrapper.fireValueChangedEvent(ReadOnlyObjectWrapper.java:102)
	at javafx.scene.Node$ReadOnlyObjectWrapperManualFire.fireSuperValueChangedEvent(Node.java:831)
	at javafx.scene.Node.invalidatedScenes(Node.java:883)
	at javafx.scene.Node.setScenes(Node.java:921)
	at javafx.scene.Parent$1.onChanged(Parent.java:269)
	at com.sun.javafx.collections.TrackableObservableList.lambda$new$19(TrackableObservableList.java:45)
	at com.sun.javafx.collections.ListListenerHelper$Generic.fireValueChangedEvent(ListListenerHelper.java:329)
	at com.sun.javafx.collections.ListListenerHelper.fireValueChangedEvent(ListListenerHelper.java:73)
	at javafx.collections.ObservableListBase.fireChange(ObservableListBase.java:233)
	at javafx.collections.ListChangeBuilder.commit(ListChangeBuilder.java:482)
	at javafx.collections.ListChangeBuilder.endChange(ListChangeBuilder.java:541)
	at javafx.collections.ObservableListBase.endChange(ObservableListBase.java:205)
	at javafx.collections.ModifiableObservableListBase.setAll(ModifiableObservableListBase.java:90)
	at com.sun.javafx.collections.VetoableListDecorator.setAll(VetoableListDecorator.java:116)
	at com.sun.javafx.collections.VetoableListDecorator.setAll(VetoableListDecorator.java:108)
	at com.sun.javafx.scene.control.skin.TabPaneSkin$TabContentRegion.updateContent(TabPaneSkin.java:1581)
	at com.sun.javafx.scene.control.skin.TabPaneSkin$TabContentRegion.lambda$new$491(TabPaneSkin.java:1548)
	at javafx.beans.WeakInvalidationListener.invalidated(WeakInvalidationListener.java:83)
	at com.sun.javafx.binding.ExpressionHelper$SingleInvalidation.fireValueChangedEvent(ExpressionHelper.java:137)
	at com.sun.javafx.binding.ExpressionHelper.fireValueChangedEvent(ExpressionHelper.java:81)
	at javafx.beans.property.ObjectPropertyBase.fireValueChangedEvent(ObjectPropertyBase.java:105)
	at javafx.beans.property.ObjectPropertyBase.markInvalid(ObjectPropertyBase.java:112)
	at javafx.beans.property.ObjectPropertyBase.set(ObjectPropertyBase.java:146)
	at javafx.scene.control.Tab.setContent(Tab.java:344)
	at org.jabref.gui.entryeditor.fileannotationtab.FileAnnotationTab.initialize(FileAnnotationTab.java:35)
	at org.jabref.gui.entryeditor.EntryEditorTab.notifyAboutFocus(EntryEditorTab.java:23)
	at org.jabref.gui.entryeditor.EntryEditor.lambda$new$1(EntryEditor.java:198)
	at org.fxmisc.easybind.EasyBind.lambda$subscribe$12(EasyBind.java:263)
	at com.sun.javafx.binding.ExpressionHelper$Generic.fireValueChangedEvent(ExpressionHelper.java:361)
	at com.sun.javafx.binding.ExpressionHelper.fireValueChangedEvent(ExpressionHelper.java:81)
	at javafx.beans.property.ReadOnlyObjectPropertyBase.fireValueChangedEvent(ReadOnlyObjectPropertyBase.java:74)
	at javafx.beans.property.ReadOnlyObjectWrapper.fireValueChangedEvent(ReadOnlyObjectWrapper.java:102)
	at javafx.beans.property.ObjectPropertyBase.markInvalid(ObjectPropertyBase.java:112)
	at javafx.beans.property.ObjectPropertyBase.set(ObjectPropertyBase.java:146)
	at javafx.scene.control.SelectionModel.setSelectedItem(SelectionModel.java:102)
	at javafx.scene.control.TabPane$TabPaneSelectionModel.select(TabPane.java:717)
	at javafx.scene.control.TabPane$TabPaneSelectionModel.select(TabPane.java:735)
	at javafx.scene.control.TabPane$TabPaneSelectionModel.select(TabPane.java:656)
	at com.sun.javafx.scene.control.behavior.TabPaneBehavior.selectTab(TabPaneBehavior.java:122)
	at com.sun.javafx.scene.control.skin.TabPaneSkin$TabHeaderSkin$5.handle(TabPaneSkin.java:1332)
	at com.sun.javafx.scene.control.skin.TabPaneSkin$TabHeaderSkin$5.handle(TabPaneSkin.java:1317)
	at com.sun.javafx.event.CompositeEventHandler.dispatchBubblingEvent(CompositeEventHandler.java:86)
	at com.sun.javafx.event.EventHandlerManager.dispatchBubblingEvent(EventHandlerManager.java:238)
	at com.sun.javafx.event.EventHandlerManager.dispatchBubblingEvent(EventHandlerManager.java:191)
	at com.sun.javafx.event.CompositeEventDispatcher.dispatchBubblingEvent(CompositeEventDispatcher.java:59)
	at com.sun.javafx.event.BasicEventDispatcher.dispatchEvent(BasicEventDispatcher.java:58)
	at com.sun.javafx.event.EventDispatchChainImpl.dispatchEvent(EventDispatchChainImpl.java:114)
	at com.sun.javafx.event.BasicEventDispatcher.dispatchEvent(BasicEventDispatcher.java:56)
	at com.sun.javafx.event.EventDispatchChainImpl.dispatchEvent(EventDispatchChainImpl.java:114)
	at com.sun.javafx.event.BasicEventDispatcher.dispatchEvent(BasicEventDispatcher.java:56)
	at com.sun.javafx.event.EventDispatchChainImpl.dispatchEvent(EventDispatchChainImpl.java:114)
	at com.sun.javafx.event.BasicEventDispatcher.dispatchEvent(BasicEventDispatcher.java:56)
	at com.sun.javafx.event.EventDispatchChainImpl.dispatchEvent(EventDispatchChainImpl.java:114)
	at com.sun.javafx.event.BasicEventDispatcher.dispatchEvent(BasicEventDispatcher.java:56)
	at com.sun.javafx.event.EventDispatchChainImpl.dispatchEvent(EventDispatchChainImpl.java:114)
	at com.sun.javafx.event.EventUtil.fireEventImpl(EventUtil.java:74)
	at com.sun.javafx.event.EventUtil.fireEvent(EventUtil.java:54)
	at javafx.event.Event.fireEvent(Event.java:198)
	at javafx.scene.Scene$MouseHandler.process(Scene.java:3757)
	at javafx.scene.Scene$MouseHandler.access$1500(Scene.java:3485)
	at javafx.scene.Scene.impl_processMouseEvent(Scene.java:1762)
	at javafx.scene.Scene$ScenePeerListener.mouseEvent(Scene.java:2494)
	at com.sun.javafx.tk.quantum.EmbeddedScene.lambda$null$294(EmbeddedScene.java:256)
	at java.security.AccessController.doPrivileged(Native Method)
	at com.sun.javafx.tk.quantum.EmbeddedScene.lambda$mouseEvent$295(EmbeddedScene.java:244)
	at com.sun.javafx.application.PlatformImpl.lambda$null$173(PlatformImpl.java:295)
	at java.security.AccessController.doPrivileged(Native Method)
	at com.sun.javafx.application.PlatformImpl.lambda$runLater$174(PlatformImpl.java:294)
	at com.sun.glass.ui.InvokeLaterDispatcher$Future.run(InvokeLaterDispatcher.java:95)
	at com.sun.glass.ui.win.WinApplication._runLoop(Native Method)
	at com.sun.glass.ui.win.WinApplication.lambda$null$148(WinApplication.java:191)
	at java.lang.Thread.run(Thread.java:745)
  1. When I add a new file in the general tab and switch to the file annotation tab, this new file is not accessible there. I need to switch to a new entry or re-open the entry editor to make it visible.

  2. When I click the copy annotations button without having an annotation selected, I get a NullPointerException:

Uncaught exception Occurred in Thread[JavaFX Application Thread,5,main]
java.lang.RuntimeException: java.lang.reflect.InvocationTargetException
	at javafx.fxml.FXMLLoader$MethodHandler.invoke(FXMLLoader.java:1774)
	at javafx.fxml.FXMLLoader$ControllerMethodEventHandler.handle(FXMLLoader.java:1657)
	at com.sun.javafx.event.CompositeEventHandler.dispatchBubblingEvent(CompositeEventHandler.java:86)
	at com.sun.javafx.event.EventHandlerManager.dispatchBubblingEvent(EventHandlerManager.java:238)
	at com.sun.javafx.event.EventHandlerManager.dispatchBubblingEvent(EventHandlerManager.java:191)
	at com.sun.javafx.event.CompositeEventDispatcher.dispatchBubblingEvent(CompositeEventDispatcher.java:59)
	at com.sun.javafx.event.BasicEventDispatcher.dispatchEvent(BasicEventDispatcher.java:58)
	at com.sun.javafx.event.EventDispatchChainImpl.dispatchEvent(EventDispatchChainImpl.java:114)
	at com.sun.javafx.event.BasicEventDispatcher.dispatchEvent(BasicEventDispatcher.java:56)
	at com.sun.javafx.event.EventDispatchChainImpl.dispatchEvent(EventDispatchChainImpl.java:114)
	at com.sun.javafx.event.BasicEventDispatcher.dispatchEvent(BasicEventDispatcher.java:56)
	at com.sun.javafx.event.EventDispatchChainImpl.dispatchEvent(EventDispatchChainImpl.java:114)
	at com.sun.javafx.event.BasicEventDispatcher.dispatchEvent(BasicEventDispatcher.java:56)
	at com.sun.javafx.event.EventDispatchChainImpl.dispatchEvent(EventDispatchChainImpl.java:114)
	at com.sun.javafx.event.BasicEventDispatcher.dispatchEvent(BasicEventDispatcher.java:56)
	at com.sun.javafx.event.EventDispatchChainImpl.dispatchEvent(EventDispatchChainImpl.java:114)
	at com.sun.javafx.event.BasicEventDispatcher.dispatchEvent(BasicEventDispatcher.java:56)
	at com.sun.javafx.event.EventDispatchChainImpl.dispatchEvent(EventDispatchChainImpl.java:114)
	at com.sun.javafx.event.BasicEventDispatcher.dispatchEvent(BasicEventDispatcher.java:56)
	at com.sun.javafx.event.EventDispatchChainImpl.dispatchEvent(EventDispatchChainImpl.java:114)
	at com.sun.javafx.event.BasicEventDispatcher.dispatchEvent(BasicEventDispatcher.java:56)
	at com.sun.javafx.event.EventDispatchChainImpl.dispatchEvent(EventDispatchChainImpl.java:114)
	at com.sun.javafx.event.EventUtil.fireEventImpl(EventUtil.java:74)
	at com.sun.javafx.event.EventUtil.fireEvent(EventUtil.java:49)
	at javafx.event.Event.fireEvent(Event.java:198)
	at javafx.scene.Node.fireEvent(Node.java:8413)
	at javafx.scene.control.Button.fire(Button.java:185)
	at com.sun.javafx.scene.control.behavior.ButtonBehavior.mouseReleased(ButtonBehavior.java:182)
	at com.sun.javafx.scene.control.skin.BehaviorSkinBase$1.handle(BehaviorSkinBase.java:96)
	at com.sun.javafx.scene.control.skin.BehaviorSkinBase$1.handle(BehaviorSkinBase.java:89)
	at com.sun.javafx.event.CompositeEventHandler$NormalEventHandlerRecord.handleBubblingEvent(CompositeEventHandler.java:218)
	at com.sun.javafx.event.CompositeEventHandler.dispatchBubblingEvent(CompositeEventHandler.java:80)
	at com.sun.javafx.event.EventHandlerManager.dispatchBubblingEvent(EventHandlerManager.java:238)
	at com.sun.javafx.event.EventHandlerManager.dispatchBubblingEvent(EventHandlerManager.java:191)
	at com.sun.javafx.event.CompositeEventDispatcher.dispatchBubblingEvent(CompositeEventDispatcher.java:59)
	at com.sun.javafx.event.BasicEventDispatcher.dispatchEvent(BasicEventDispatcher.java:58)
	at com.sun.javafx.event.EventDispatchChainImpl.dispatchEvent(EventDispatchChainImpl.java:114)
	at com.sun.javafx.event.BasicEventDispatcher.dispatchEvent(BasicEventDispatcher.java:56)
	at com.sun.javafx.event.EventDispatchChainImpl.dispatchEvent(EventDispatchChainImpl.java:114)
	at com.sun.javafx.event.BasicEventDispatcher.dispatchEvent(BasicEventDispatcher.java:56)
	at com.sun.javafx.event.EventDispatchChainImpl.dispatchEvent(EventDispatchChainImpl.java:114)
	at com.sun.javafx.event.BasicEventDispatcher.dispatchEvent(BasicEventDispatcher.java:56)
	at com.sun.javafx.event.EventDispatchChainImpl.dispatchEvent(EventDispatchChainImpl.java:114)
	at com.sun.javafx.event.BasicEventDispatcher.dispatchEvent(BasicEventDispatcher.java:56)
	at com.sun.javafx.event.EventDispatchChainImpl.dispatchEvent(EventDispatchChainImpl.java:114)
	at com.sun.javafx.event.BasicEventDispatcher.dispatchEvent(BasicEventDispatcher.java:56)
	at com.sun.javafx.event.EventDispatchChainImpl.dispatchEvent(EventDispatchChainImpl.java:114)
	at com.sun.javafx.event.BasicEventDispatcher.dispatchEvent(BasicEventDispatcher.java:56)
	at com.sun.javafx.event.EventDispatchChainImpl.dispatchEvent(EventDispatchChainImpl.java:114)
	at com.sun.javafx.event.BasicEventDispatcher.dispatchEvent(BasicEventDispatcher.java:56)
	at com.sun.javafx.event.EventDispatchChainImpl.dispatchEvent(EventDispatchChainImpl.java:114)
	at com.sun.javafx.event.EventUtil.fireEventImpl(EventUtil.java:74)
	at com.sun.javafx.event.EventUtil.fireEvent(EventUtil.java:54)
	at javafx.event.Event.fireEvent(Event.java:198)
	at javafx.scene.Scene$MouseHandler.process(Scene.java:3757)
	at javafx.scene.Scene$MouseHandler.access$1500(Scene.java:3485)
	at javafx.scene.Scene.impl_processMouseEvent(Scene.java:1762)
	at javafx.scene.Scene$ScenePeerListener.mouseEvent(Scene.java:2494)
	at com.sun.javafx.tk.quantum.EmbeddedScene.lambda$null$294(EmbeddedScene.java:256)
	at java.security.AccessController.doPrivileged(Native Method)
	at com.sun.javafx.tk.quantum.EmbeddedScene.lambda$mouseEvent$295(EmbeddedScene.java:244)
	at com.sun.javafx.application.PlatformImpl.lambda$null$173(PlatformImpl.java:295)
	at java.security.AccessController.doPrivileged(Native Method)
	at com.sun.javafx.application.PlatformImpl.lambda$runLater$174(PlatformImpl.java:294)
	at com.sun.glass.ui.InvokeLaterDispatcher$Future.run(InvokeLaterDispatcher.java:95)
	at com.sun.glass.ui.win.WinApplication._runLoop(Native Method)
	at com.sun.glass.ui.win.WinApplication.lambda$null$148(WinApplication.java:191)
	at java.lang.Thread.run(Thread.java:745)
Caused by: java.lang.reflect.InvocationTargetException
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:498)
	at sun.reflect.misc.Trampoline.invoke(MethodUtil.java:71)
	at sun.reflect.GeneratedMethodAccessor2.invoke(Unknown Source)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:498)
	at sun.reflect.misc.MethodUtil.invoke(MethodUtil.java:275)
	at javafx.fxml.FXMLLoader$MethodHandler.invoke(FXMLLoader.java:1769)
	... 67 more
Caused by: java.lang.NullPointerException
	at org.jabref.gui.entryeditor.fileannotationtab.FileAnnotationTabViewModel.copyCurrentAnnotation(FileAnnotationTabViewModel.java:82)
	at org.jabref.gui.entryeditor.fileannotationtab.FileAnnotationTabController.copy(FileAnnotationTabController.java:87)
	... 77 more
  1. When I click the reload annotations button, nothing happens (even though I added an annotation to the document and saved it). There are no errors in the log.

@Siedlerchr
Copy link
Member

I fixed one exception, however, the other is a classical NPE, because the ObjectProperty returns null, because it is not intialized. You should either define an empty File Annotation or disable the copy dialog @lynyus

@LinusDietz
Copy link
Member Author

@JabRef/developers Please have a look if this PR is ready to be merged.

}

private FileAnnotationViewModel getCurrentAnnotation() {
//TODO: It has to be initialized! NPE
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove comment if it is resolved

@LinusDietz
Copy link
Member Author

I will merge this now despite minor issues: #3110 and #3109.

@LinusDietz LinusDietz merged commit 455965e into master Aug 16, 2017
@LinusDietz LinusDietz deleted the javafx-fileannotationtab branch August 16, 2017 08:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
external files status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers ui
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Switching from FileAnnotationTab causes high CPU load Problems with PDF annotations
4 participants