-
-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
Update PreviewView
to listen for library search queries
#11659
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a small comment
vBox.getChildren().add(label); | ||
vBox.getChildren().add(preview); | ||
this.setGraphic(vBox); | ||
fieldValueLabel.setText(fieldValue + "\n"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can be moved to the if block
Please swap the conditions - typically, one should have the positive case first. Alternatively, "fail fast". Keep the negative thing, but then do "return this;" and then without else
do the rest.
The indent of line 37 "this.setGraphic" is off
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can be moved to the if block
In both cases it should be updated, as it is also used in the tooltipContent
line 27.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, wow, something which is not directly seeable while browsing through the github diff 😅
if (!registered) { | ||
stateManager.activeSearchQuery(SearchType.NORMAL_SEARCH).addListener(listener); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we also remove the activeSearchQuery
completely from the state manager?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is still used by GlobalSearchBar
because there is only one bar shared between the open libraries.
The build for this PR is no longer available. Please visit https://builds.jabref.org/main/ for the latest build. |
PreviewView
to listen for library search queries instead of theStateManager
property.shouldShowPreviewEntryTableTooltip
was disabled.Mandatory checks
CHANGELOG.md
described in a way that is understandable for the average user (if applicable)